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

Stream functions in separate files #453

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

buggywhip
Copy link
Contributor

@buggywhip buggywhip commented Oct 15, 2018

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

@buggywhip buggywhip force-pushed the streams-functions_in_separate_files branch 5 times, most recently from d054762 to 8d0c2cb Compare October 16, 2018 02:46
@buggywhip buggywhip force-pushed the streams-functions_in_separate_files branch from 8d0c2cb to b34750c Compare October 16, 2018 03:14
@buggywhip buggywhip force-pushed the streams-functions_in_separate_files branch from b34750c to a0151be Compare October 16, 2018 03:29
@buggywhip buggywhip requested review from karel-m and sjaeckel October 16, 2018 05:38
@karel-m
Copy link
Member

karel-m commented Nov 6, 2018

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
Copy link
Member

karel-m commented Nov 6, 2018

plus perhaps also get rid of sober128tab.c

@buggywhip
Copy link
Contributor Author

buggywhip commented Nov 11, 2018 via email

@karel-m
Copy link
Member

karel-m commented Nov 30, 2018

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).

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.

2 participants