Skip to content

cbor: allow user to control nesting#329

Merged
kaczmarczyck merged 3 commits intogoogle:developfrom
daviddrysdale:cbor-nested
Jun 18, 2021
Merged

cbor: allow user to control nesting#329
kaczmarczyck merged 3 commits intogoogle:developfrom
daviddrysdale:cbor-nested

Conversation

@daviddrysdale
Copy link
Copy Markdown
Contributor

@daviddrysdale daviddrysdale commented Jun 17, 2021

  • Make the default read/write entrypoints allow infinite nesting.
  • Add {read,write}_nested() entrypoints that allow the crate user to
    control the depth of nesting that's allowed.
  • Along the way, convert the write[_nested] variants to return a
    Result<(), EncoderError> rather than a bool. This exposes
    more failure information (and forces the caller to take notice
    of those tailures), and allows use of the ? operator.

@google-cla google-cla bot added the cla: yes label Jun 17, 2021
 - Make the default read/write entrypoints allow infinite nesting.
 - Add {read,write}_nested() entrypoints that allow the crate user to
   control the depth of nesting that's allowed.
 - Along the way, convert the write[_nested] variants to return a
   `Result<(), EncoderError>` rather than a bool.  This exposes
   more failure information (and forces the caller to take notice
   of those tailures), and allows use of the ? operator.
@daviddrysdale daviddrysdale changed the title cbor: add {read,write}_nested() entrypoints cbor: allow user to control nesting Jun 18, 2021
Copy link
Copy Markdown
Collaborator

@kaczmarczyck kaczmarczyck left a comment

Choose a reason for hiding this comment

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

I'd propose a change in cbor_read and cbor_write that works specifically for CTAP. I marked the lines that would need changes for convenience.

Copy link
Copy Markdown
Collaborator

@kaczmarczyck kaczmarczyck left a comment

Choose a reason for hiding this comment

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

Perfect! The code is cleaner now. Binary size also didn't change much (my current quest :).

@daviddrysdale
Copy link
Copy Markdown
Contributor Author

Should I squash into a single commit or do you normally squash-and-merge?

@kaczmarczyck kaczmarczyck merged commit 0287a09 into google:develop Jun 18, 2021
@kaczmarczyck
Copy link
Copy Markdown
Collaborator

I'll just squash-and-merge.

@daviddrysdale daviddrysdale deleted the cbor-nested branch June 18, 2021 17:57
ia0 added a commit to ia0/OpenSK that referenced this pull request Aug 11, 2021
ia0 added a commit that referenced this pull request Aug 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants