laurencelundblade / QCBOR

Comprehensive, powerful, commercial-quality CBOR encoder/ decoder that is still suited for small devices.
Other
181 stars 47 forks source link

QCBOR should have QCBORDecode_SetError() #210

Closed osibert closed 5 months ago

osibert commented 6 months ago

In writing decoders for some applications, it is often necessary to perform similar checks of decoded values in many different parts of the code. Such checks (e.g., integer range, valid values, text/byte string size checks, array size checks) can conveniently be implemented as thin wrappers on the underlying QCBORDecode_GetXXX and related functions, but there appears to be no way to indicate to the QCBOR decoder that a check has failed, stopping subsequent decoding and preserving the error value and context, unlike the regular spiffy decode functions which halt further decoding when they encounter a CBOR syntax error.

Would QCBORDecode_SetError(), perhaps in combination with some additional QCBORError values, be a reasonable way to facilitate this end?

Is there a straightforward (but of course undocumented and subject to change) way to do this by accessing the decoder structures directly? I have been reluctant to dive into the code without a sanity check on the concept.

I imagine defining QCBORError values like QCBOR_ERR_INVALID_VALUE, QCBOR_ERR_VALUE_OUT_OF_RANGE, QCBOR_ERR_INVALID_TYPE, QCBOR_ERR_STRING_TOO_LARGE, QCBOR_ERR_STRING_TOO_SHORT, QCBOR_ERR_INVALID_ARRAY_SIZE, etc.

My current solution wraps the whole decoding process with a thin layer of wrappers and maintains a separate copy of the error state in the wrapper object, but that seems a clumsy approach at best.

I see Issue #99 starts to address this objective, but it's a really specialized aspect of value checking (that said, I did end up writing those very functions in my set of wrappers).

Thank you for your consideration.

laurencelundblade commented 6 months ago

Sounds like a good idea to me.

Pretty sure you can just set QCBORDecodeContext.uLastError to a value other than QCBOR_SUCCESS and it will do what you want.

   if(pMe->uLastError != QCBOR_SUCCESS) {
      return;
   }

occurs about 50 times in the code, usually at the beginning of a decode method.

I will probably reserve error code 200-255 for end-user use.

I will probably implement this in the next month or so. Testing seems like the hardest part. :-)

Thanks for the suggestion!

osibert commented 6 months ago

Laurence, thanks so much for the quick and positive response. I'm glad you liked the idea and that it arrived at a convenient time. I shall apply your suggested workaround promptly.

I squandered quite a few hours prototyping with libcbor and then TinyCBOR before I discovered QCBOR, which has proved much easier to work with. I've been quite pleased with your design approach and the care taken to carve out pieces that may be unneeded in some environments. I've been looking forward to using CBOR as a system architecture enabler for many years, and it finally seems like the supporting parts are ready.

I'm probably going to end up doing a training presentation for my team on the CBOR format and the QCBOR library; would you be interested in reviewing that and making it available to others? Or, alternatively, might you have some material like that which I could update and revise? My client for this project is committed in principle to open sourcing this stuff, though it's a new experience for them. I have no idea whether it can really come to pass, but I certainly will try to make it happen. The more worked examples of CBOR there are, the better off we will be.

Thanks again. Olin Sibert

On Thu, Feb 29, 2024 at 7:49 PM Laurence Lundblade @.***> wrote:

Sounds like a good idea to me.

Pretty sure you can just set QCBORDecodeContext.uLastError to a value other than QCBOR_SUCCESS and it will do what you want.

if(pMe->uLastError != QCBOR_SUCCESS) { return; }

occurs about 50 times in the code, usually at the beginning of a decode method.

I will probably reserve error code 200-255 for end-user use.

I will probably implement this in the next month or so. Testing seems like the hardest part. :-)

Thanks for the suggestion!

— Reply to this email directly, view it on GitHub https://github.com/laurencelundblade/QCBOR/issues/210#issuecomment-1972458189, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARLDCC76EGECJIIN6J72ZGTYV73DXAVCNFSM6AAAAABEAT73OKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZSGQ2TQMJYHE . You are receiving this because you authored the thread.Message ID: @.***>

laurencelundblade commented 6 months ago

Yes, happy to review. I do have some materials on CBOR (not QCBOR). Let's switch to email for that discussion lgl@island-resort.com.

laurencelundblade commented 5 months ago

Fixed by #214. Eventually a QCBOR 1.3 will be released. For now this is just the tip in GitHub.