laurencelundblade / QCBOR

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

Documentation on and example of preferred way to iterate a variable-length array #233

Closed BrianSipos closed 1 month ago

BrianSipos commented 2 months ago

The current inline docs for spiffy array decoding starting below include description of its behavior but not an explicit example of how to efficiently iterate through an array with a variable (at the encoder) length.

https://github.com/laurencelundblade/QCBOR/blob/5db34da44011ef8dfa586134fe84ddcda6ec99b8/inc/qcbor/qcbor_spiffy_decode.h#L651-L652

The current docs state that

decoding will not proceed past the end or the array

and I have been using the following pattern which uses that logic, but I'm not sure if this is the most efficient way because the PeekNext function wastes decoding effort just to tell end-of-array. Is this the recommended way to efficiently iterate an array when using spiffy decoder functions? If so, it could be useful to include this in the docs or even in an example (I wasn't able to find a decoding example like this).

    QCBORDecodeContext asbdec;
    QCBORItem asbitem;
...

    QCBORDecode_EnterArray(&asbdec, NULL);
    while (QCBOR_SUCCESS == QCBORDecode_PeekNext(&asbdec, &asbitem))
    {
        uint64_t num;
        QCBORDecode_GetUInt64(&asbdec, &num);
...
    }
    QCBORDecode_ExitArray(&asbdec);
BrianSipos commented 2 months ago

I initially thought that PeekNext() could be used with a NULL item argument but that is not the case currently. Maybe the API for PeekNext could be updated to have a NULL item return either success or failure if there is any item to decode without caring about the contents of the item (or any recursion on it or its tags).

laurencelundblade commented 2 months ago

Also, if you just want to skip an array entirely, the simplest way is QCBORDecode_VGetNextConsume().

laurencelundblade commented 2 months ago

VGetNext() is the way to iterate items in an array, PeekNext() is less efficient than VGetNext() and doesn't advance the cursor. I would think your loop above doesn't terminate.

Appreciate you commenting, so I can improve the documentation.

BrianSipos commented 2 months ago

I should mention that the second ellipsis (one within the loop) actually reads the items from the array, so that's how the cursor advances.

BrianSipos commented 2 months ago

I updated the example in the ticket to make clear also that I'm using spiffy decoder functions, so I don't want the cursor to advance when checking end-of-array because my GetUInt64() will need to read each item of the array. In this case it's known to be an array of uint items.

laurencelundblade commented 2 months ago

OK. Understood. Below should work. When GetError() returns QCBOR_ERR_NO_MORE_ITEMS you've hit the non-error end of the loop. There might be other input errors so you have to stop on any error. This does use the cursor as that's the only way to traverse an array.

    QCBORDecode_EnterArray(&asbdec, NULL);
    while (1)
    {
        uint64_t num;
        QCBORDecode_GetUInt64(&asbdec, &num);
        if(QCBORDecode_GetError(&asbdec)) {
            break;
        }
...
    }
    QCBORDecode_ExitArray(&asbdec);
laurencelundblade commented 2 months ago

Hi Brian, did this solve it for you? Appreciate if you could confirm so I can merge these documentation fixes.

BrianSipos commented 2 months ago

That technique of checking error after getting each item of the array works fine for my use case, thank you. Is this still an efficient technique if the items of the array are more complex structure (e.g. maps)?

Would this also be an acceptable practice that properly exits the loop after the last map contained in an array is read? From looking in the source it seems like yes.

    QCBORDecode_EnterArray(&asbdec, NULL);
    while (1)
    {
        QCBORDecode_EnterMap(&asbdec, NULL);
        if(QCBORDecode_GetError(&asbdec)) {
            break;
        }
...
    }
    QCBORDecode_ExitArray(&asbdec);
laurencelundblade commented 2 months ago

If you run the loop by the number of items in the array (don't pass NULL to EnterArray to get the item with the array count), that works too, but only for arrays encoded with deterministic lengths. The error check works for both.

There's probably no difference in efficiency. QCBORDecode_GetError() is an inline function that just accesses a single variable so that error check is not really any more costly than any other way of detecting the end of an array.

Yes, the code you have above will work.

BrianSipos commented 2 months ago

Thanks for all of this info. I think the doc updates are good and can eventually close this ticket.

laurencelundblade commented 1 month ago

Doc updates made...