Skip to content

Conversation

@sjaeckel
Copy link
Member

@sjaeckel sjaeckel commented Dec 6, 2017

Minor cleanup&lil'feature PR

The only thing really worth a review is the newly introduced copy_or_zeromem()

The rest is just cosmetics.

Reviews are as always very welcome!

@karel-m
Copy link
Member

karel-m commented Dec 9, 2017

Ad copy_or_zeromem

  • I am not sure if we want it to be part of public API
  • if it'll will be part of public API then the name is quite mysterious (I do not have a better name)
  • if copy_or_zeromem is handy in CCM then it should be perhaps also used in other encauth cases

@sjaeckel
Copy link
Member Author

if copy_or_zeromem is handy in CCM then it should be perhaps also used in other encauth cases

I factored this out as I use it in my SIV PR as well, but I thought it doesn't really belong there, so I created this PR with other changes that belong to the features introduced there

I am not sure if we want it to be part of public API

I'll make it private for now

if it'll will be part of public API then the name is quite mysterious (I do not have a better name)

I think the name is okay'ish... probably one of the following would be better, not sure though:

  1. mem_copy_or_zero
  2. memcpy_or_zero
  3. copy_or_zero
  4. copy_or_clean

@karel-m
Copy link
Member

karel-m commented Dec 10, 2017

If you make it private then no need to care about the name.

@sjaeckel
Copy link
Member Author

sjaeckel commented Dec 11, 2017

If you make it private then no need to care about the name.

well I prefer to have a proper name now so we don't have to think about it in case we want to make it public ;)

Edit: I think the current name is fine and represents what the function does. Therefore we could even make it public.

@karel-m
Copy link
Member

karel-m commented Dec 11, 2017

IIUC we are doing: "copy src to dest or zero dest (all that in constant time)" which sounds to me more like copy_or_zero_dest but I can live with copy_or_zeromem as well - I leave it up to you.

Another thing (UPDATED):

- coz &= 0x1;
+ if (coz != 0) coz = 1;

so that unexpected/accidental coz = 2 means zeromem (accidentally zeroing is better that copying)

Copy link
Member

@karel-m karel-m left a comment

Choose a reason for hiding this comment

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

👍

@sjaeckel sjaeckel merged commit 8ef3b9d into develop Dec 16, 2017
@sjaeckel sjaeckel deleted the minor_cleanup branch December 16, 2017 23:43
@sjaeckel sjaeckel added this to the next milestone Jan 23, 2018
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.

3 participants