Minor cleanup #335

Merged
sjaeckel merged 12 commits from minor_cleanup into develop 2017-12-16 17:43:36 -06:00
sjaeckel commented 2017-12-06 17:08:19 -06:00 (Migrated from github.com)

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!

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 commented 2017-12-09 13:09:41 -06:00 (Migrated from github.com)

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
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 commented 2017-12-10 06:08:38 -06:00 (Migrated from github.com)

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
> 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 commented 2017-12-10 10:09:21 -06:00 (Migrated from github.com)

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

If you make it private then no need to care about the name.
sjaeckel commented 2017-12-11 10:02:40 -06:00 (Migrated from github.com)

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.

> 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 commented 2017-12-11 12:22:34 -06:00 (Migrated from github.com)

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)

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): ```diff - coz &= 0x1; + if (coz != 0) coz = 1; ``` so that unexpected/accidental `coz = 2` means zeromem (accidentally zeroing is better that copying)
karel-m (Migrated from github.com) approved these changes 2017-12-16 14:36:01 -06:00
karel-m (Migrated from github.com) left a comment

👍

👍
Sign in to join this conversation.
No reviewers
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-libtom/libtomcrypt!335
No description provided.