gh-libtom/libtomcrypt
Watch
1
0
Fork
You've already forked libtomcrypt
 0
Code Issues 42 Pull requests 17 Releases 9 Wiki Activity

Stream functions in separate files #453

Open
buggywhip wants to merge 2 commits from streams-functions_in_separate_files into develop
pull from: streams-functions_in_separate_files
merge into: gh-libtom:develop
Conversation 6 Commits 2 Files changed 32 +2420 -1399
buggywhip commented 2018-10-14 20:52:41 -05:00 (Migrated from github.com)

Some stream ciphers had multiple functions in a file. This PR breaks each callable function out into separate files. Common declarations and some definitions unique to that cipher will be found in a separate local .h file for that cipher. Some guards were added to avoid warnings. Leading underscores were added to the names of sober128's helper functions.

Checklist

  • [ N/A ] documentation is added or updated
  • [ N/A ] tests are added or updated
Some stream ciphers had multiple functions in a file. This PR breaks each callable function out into separate files. Common declarations and some definitions unique to that cipher will be found in a separate local .h file for that cipher. Some guards were added to avoid warnings. Leading underscores were added to the names of sober128's helper functions. ### Checklist * [ N/A ] documentation is added or updated * [ N/A ] tests are added or updated
sjaeckel (Migrated from github.com) reviewed 2018-10-14 20:52:41 -05:00
karel-m (Migrated from github.com) reviewed 2018-10-14 20:52:41 -05:00
karel-m commented 2018-11-06 15:47:25 -06:00 (Migrated from github.com)

I suggest just keeping separate NN_memory, NN_test, NN_keystream and perhaps NN_done like:

Old style                         This PR                                  Karel's suggestion 
================================= =======================================  ====================================
rabbit/rabbit.c                                                            rabbit/rabbit.c
                                  rabbit/rabbit_common.h                  
                                  rabbit/rabbit_setup.c                   
                                  rabbit/rabbit_setiv.c                   
                                  rabbit/rabbit_crypt.c                   
                                  rabbit/rabbit_done.c                    +rabbit/rabbit_done.c
                                  rabbit/rabbit_keystream.c               +rabbit/rabbit_keystream.c
rabbit/rabbit_memory.c            rabbit/rabbit_memory.c                   rabbit/rabbit_memory.c
                                  rabbit/rabbit_test.c                    +rabbit/rabbit_test.c
================================= =======================================  ====================================
rc4/rc4_stream.c                                                           rc4/rc4_stream.c
                                  rc4/rc4_stream_setup.c                  
                                  rc4/rc4_stream_crypt.c                   
                                  rc4/rc4_stream_done.c                   +rc4/rc4_stream_done.c
                                  rc4/rc4_stream_keystream.c              +rc4/rc4_stream_keystream.c
rc4/rc4_stream_memory.c           rc4/rc4_stream_memory.c                  rc4/rc4_stream_memory.c
rc4/rc4_test.c                    rc4/rc4_test.c                           rc4/rc4_test.c
================================= =======================================  ====================================
sober128/sober128_stream.c                                                 sober128/sober128_stream.c
                                  sober128/sober128_stream_common.h       
                                  sober128/sober128_stream_setup.c        
                                  sober128/sober128_stream_setiv.c        
                                  sober128/sober128_stream_crypt.c        
                                  sober128/sober128_stream_done.c         +sober128/sober128_stream_done.c
                                  sober128/sober128_stream_keystream.c    +sober128/sober128_stream_keystream.c
sober128/sober128_stream_memory.c sober128/sober128_stream_memory.c        sober128/sober128_stream_memory.c
sober128/sober128_test.c          sober128/sober128_test.c                 sober128/sober128_test.c
sober128/sober128tab.c            sober128/sober128tab.c                   sober128/sober128tab.c
================================= =======================================  ====================================
sosemanuk/sosemanuk.c                                                      sosemanuk/sosemanuk.c
                                  sosemanuk/sosemanuk_common.h            
                                  sosemanuk/sosemanuk_setup.c             
                                  sosemanuk/sosemanuk_setiv.c             
                                  sosemanuk/sosemanuk_crypt.c             
                                  sosemanuk/sosemanuk_done.c              +sosemanuk/sosemanuk_done.c
                                  sosemanuk/sosemanuk_keystream.c         +sosemanuk/sosemanuk_keystream.c
sosemanuk/sosemanuk_memory.c      sosemanuk/sosemanuk_memory.c             sosemanuk/sosemanuk_memory.c
sosemanuk/sosemanuk_test.c        sosemanuk/sosemanuk_test.c               sosemanuk/sosemanuk_test.c
================================= =======================================  ====================================
I suggest just keeping separate NN_memory, NN_test, NN_keystream and perhaps NN_done like: ``` Old style This PR Karel's suggestion ================================= ======================================= ==================================== rabbit/rabbit.c rabbit/rabbit.c rabbit/rabbit_common.h rabbit/rabbit_setup.c rabbit/rabbit_setiv.c rabbit/rabbit_crypt.c rabbit/rabbit_done.c +rabbit/rabbit_done.c rabbit/rabbit_keystream.c +rabbit/rabbit_keystream.c rabbit/rabbit_memory.c rabbit/rabbit_memory.c rabbit/rabbit_memory.c rabbit/rabbit_test.c +rabbit/rabbit_test.c ================================= ======================================= ==================================== rc4/rc4_stream.c rc4/rc4_stream.c rc4/rc4_stream_setup.c rc4/rc4_stream_crypt.c rc4/rc4_stream_done.c +rc4/rc4_stream_done.c rc4/rc4_stream_keystream.c +rc4/rc4_stream_keystream.c rc4/rc4_stream_memory.c rc4/rc4_stream_memory.c rc4/rc4_stream_memory.c rc4/rc4_test.c rc4/rc4_test.c rc4/rc4_test.c ================================= ======================================= ==================================== sober128/sober128_stream.c sober128/sober128_stream.c sober128/sober128_stream_common.h sober128/sober128_stream_setup.c sober128/sober128_stream_setiv.c sober128/sober128_stream_crypt.c sober128/sober128_stream_done.c +sober128/sober128_stream_done.c sober128/sober128_stream_keystream.c +sober128/sober128_stream_keystream.c sober128/sober128_stream_memory.c sober128/sober128_stream_memory.c sober128/sober128_stream_memory.c sober128/sober128_test.c sober128/sober128_test.c sober128/sober128_test.c sober128/sober128tab.c sober128/sober128tab.c sober128/sober128tab.c ================================= ======================================= ==================================== sosemanuk/sosemanuk.c sosemanuk/sosemanuk.c sosemanuk/sosemanuk_common.h sosemanuk/sosemanuk_setup.c sosemanuk/sosemanuk_setiv.c sosemanuk/sosemanuk_crypt.c sosemanuk/sosemanuk_done.c +sosemanuk/sosemanuk_done.c sosemanuk/sosemanuk_keystream.c +sosemanuk/sosemanuk_keystream.c sosemanuk/sosemanuk_memory.c sosemanuk/sosemanuk_memory.c sosemanuk/sosemanuk_memory.c sosemanuk/sosemanuk_test.c sosemanuk/sosemanuk_test.c sosemanuk/sosemanuk_test.c ================================= ======================================= ==================================== ```
karel-m commented 2018-11-06 15:49:04 -06:00 (Migrated from github.com)

plus perhaps also get rid of sober128tab.c

plus perhaps also get rid of `sober128tab.c`
buggywhip commented 2018-11-11 00:40:20 -06:00 (Migrated from github.com)

On 6Nov, 2018, at 1:47 PM, karel-m notifications@github.com wrote:
I suggest just keeping separate NN_memory, NN_test, NN_keystream and perhaps NN_done like:

[snip]

After careful consideration I'm leaning toward each callable function in its own file for reasons of consistency. IMO there is not sufficient advantage to combining some functions, each cipher differently, when some optional and sometine required functions need to be called out for linking individually. For example, the inconsistent model for...

streams combining _setup(), setiv(), and _crypt(): rabbit, sober128, sosemanuk
streams combining _setup(),and _crypt(): rc4
streams combining none: chacha, (x)salsa20

...requires calling out what you need that wasn't combined. Confusing.

Implementing the PR as written makes the decision rule consistent, "if you call it, link it".

> On 6Nov, 2018, at 1:47 PM, karel-m <notifications@github.com> wrote: > I suggest just keeping separate NN_memory, NN_test, NN_keystream and perhaps NN_done like: > [snip] After careful consideration I'm leaning toward each callable function in its own file for reasons of consistency. IMO there is not sufficient advantage to combining some functions, each cipher differently, when some optional and sometine required functions need to be called out for linking individually. For example, the inconsistent model for... streams combining _setup(), setiv(), and _crypt(): rabbit, sober128, sosemanuk streams combining _setup(),and _crypt(): rc4 streams combining none: chacha, (x)salsa20 ...requires calling out what you need that wasn't combined. Confusing. Implementing the PR as written makes the decision rule consistent, "if you call it, link it".
karel-m commented 2018-11-30 14:24:15 -06:00 (Migrated from github.com)

I see your point but to me this is "over-consistent" for no (or very little) gain.

Anyway, I am strongly against introducing new headers - <something>_common.h. I suggest either to merge those .c files sharing the same <something>_common.h header (which I prefer) or perhaps to use tomcrypt_private.h (but I somehow feel that @sjaeckel might not like it as it may not comply with the original tomcrypt_private.h idea).

I see your point but to me this is "over-consistent" for no (or very little) gain. Anyway, I am strongly against introducing new headers - `<something>_common.h`. I suggest either to merge those `.c` files sharing the same `<something>_common.h` header (which I prefer) or perhaps to use `tomcrypt_private.h` (but I somehow feel that @sjaeckel might not like it as it may not comply with the original `tomcrypt_private.h` idea).
This pull request has changes conflicting with the target branch.
  • helper.pl
  • makefile.mingw
  • makefile.msvc
  • makefile.unix
  • makefile_include.mk
  • src/stream/rabbit/rabbit.c
  • src/stream/rc4/rc4_stream.c
  • src/stream/sober128/sober128_stream.c
  • src/stream/sosemanuk/sosemanuk_crypt.c
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin 'streams-functions_in_separate_files:streams-functions_in_separate_files'
git switch 'streams-functions_in_separate_files'

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch 'develop'
git merge --no-ff 'streams-functions_in_separate_files'
git switch 'streams-functions_in_separate_files'
git rebase 'develop'
git switch 'develop'
git merge --ff-only 'streams-functions_in_separate_files'
git switch 'streams-functions_in_separate_files'
git rebase 'develop'
git switch 'develop'
git merge --no-ff 'streams-functions_in_separate_files'
git switch 'develop'
git merge --squash 'streams-functions_in_separate_files'
git switch 'develop'
git merge --ff-only 'streams-functions_in_separate_files'
git switch 'develop'
git merge 'streams-functions_in_separate_files'
git push origin 'develop'
Sign in to join this conversation.
Reviewers
No reviewers
sjaeckel
karel-m
Labels
Clear labels   
bug

   
duplicate

   
enhancement

   
feature

   
help wanted

   
improve API

   
invalid

   
needs documentation

   
not-planned
This ticket is not planned to be implemented and/or fixed from the core team.

  
rtfm

   
stale

   
support

   
wishlist
No labels
bug
duplicate
enhancement
feature
help wanted
improve API
invalid
needs documentation
not-planned
rtfm
stale
support
wishlist
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
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!453
No description provided.