Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

implemented decoding indefinite array #21

Merged
merged 3 commits into from Feb 9, 2021
Merged

Conversation

nagrawal63
Copy link
Contributor

This has the indefinite array implemented in it. Please let me know if there are any issues.

@bergzand
Copy link
Owner

Please rebase on latest master as this fixes the current CI issues in this PR.

@bergzand
Copy link
Owner

Also, please remove any commented code from your commits

@benpicco
Copy link
Contributor

@Nishchay-sopho ping 😉

@nagrawal63
Copy link
Contributor Author

nagrawal63 commented Aug 16, 2020

Yeah sure, I totally forgot about it 😅 . Will do the needful asap

@benpicco
Copy link
Contributor

I would have expected to find the implementation in src/decoder.c, not in the test case 😉

@nagrawal63
Copy link
Contributor Author

The latest commit resolves the issue #20 and also resolves indefinite map support.

Testing :
Use the function below to encode an indefinite array

static void _encode(nanocbor_encoder_t *enc)
{
    nanocbor_fmt_array_indefinite(enc);
    nanocbor_fmt_bool(enc, true);
    nanocbor_fmt_bool(enc, false);
    nanocbor_fmt_uint(enc, UINT32_MAX);
    nanocbor_fmt_int(enc, INT32_MIN);
    nanocbor_fmt_map(enc, 4);
    nanocbor_fmt_uint(enc, 8);
    nanocbor_fmt_int(enc, 30);
    nanocbor_fmt_int(enc, -30);
    nanocbor_fmt_int(enc, 500);
    nanocbor_fmt_int(enc, -500);
    nanocbor_put_tstr(enc, "this is a long string");
    nanocbor_fmt_float(enc, 0.34);
    nanocbor_put_bstr(enc, (uint8_t*)"bytez", sizeof("bytez"));
    nanocbor_fmt_null(enc);
    nanocbor_fmt_end_indefinite(enc);
}

On calling decode on this encoded buffer we get the following output:

[
  True,
  False,
  4294967295,
  -2147483648,
  {
    8: 30,
    -30: 500,
    -500: "this is a long string",
    Unsupported float: "0x62, 0x79, 0x74, 0x65, 0x7a, 0x00, ",
  },
  NULL,
],

@benpicco
Copy link
Contributor

You'll still need to do a git rebase master and solve the merge conflict :)

@benpicco
Copy link
Contributor

Lint has still some issues, most of them should be fixed if you rebase.

Maybe then @bergzand can take a look at this 😉

@nagrawal63
Copy link
Contributor Author

The lint check is giving a weird error:

/tmp/cirrus-ci-build/src/decoder.c:282:33: error: use of a signed integer operand with a binary bitwise operator [hicpp-signed-bitwise,-warnings-as-errors]
    if (_value_match_exact(it, (type << NANOCBOR_TYPE_OFFSET | NANOCBOR_SIZE_INDEFINITE)) == 1) {

More on it here

All the variables in boolean operation type, NANOCBOR_TYPE_OFFSET and NANOCBOR_SIZE_INDEFINITE are unsigned integers.

Could you kindly let me know why this is happening?

src/decoder.c Outdated
@@ -279,7 +279,7 @@ int _enter_container(nanocbor_value_t *it, nanocbor_value_t *container,
container->end = it->end;
container->remaining = 0;

if (_value_match_exact(it, type | NANOCBOR_VALUE_MASK) == 1) {
if (_value_match_exact(it, (type << NANOCBOR_TYPE_OFFSET | NANOCBOR_SIZE_INDEFINITE)) == 1) {
Copy link
Contributor

@benpicco benpicco Oct 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (_value_match_exact(it, (type << NANOCBOR_TYPE_OFFSET | NANOCBOR_SIZE_INDEFINITE)) == 1) {
if (_value_match_exact(it, (type << NANOCBOR_TYPE_OFFSET) | NANOCBOR_SIZE_INDEFINITE) == 1) {

Does that help? (it makes it at least easier to parse for humans)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lint is still not satisfied by this change

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I don't know either 😟 - maybe try casting it to (unsigned)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now lint is happy. This is so weird. The type variable was already uint8_t. 😕

Let me know if this works for you, I will then squash the commits to make them ready for merge.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it's up to @bergzand to merge this though

@bergzand
Copy link
Owner

bergzand commented Feb 9, 2021

Tested this locally, code looks good! Thanks for fixing

@bergzand bergzand merged commit 1baa179 into bergzand:master Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants