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

removes source file includes in ccnl-utils #270

Merged
merged 2 commits into from
Jun 14, 2018

Conversation

mfrey
Copy link
Collaborator

@mfrey mfrey commented Jun 13, 2018

Contribution description

This PR restructures the ccnl-utils module and introduces a include and src directory. The C-file includes have been split into a header and source file. Hence, all occurrences of e.g.

#include "ccnl-common.c"

have been replaced by

#include "ccnl-common.h"

These changes also required some customizations in the CMakeLists.txt file of the module. It also removes the USE_SIGNATURES use flag since ISO C forbids empty translation units (which was a result of splitting ccnl-crypto.c into ccnl-crypto.{h,c})

Issues/PRs references

Fixes #236

@mfrey mfrey added minor requires squashing commits need to be squashed before merge labels Jun 13, 2018
@mfrey mfrey requested review from cgundogan and blacksheeep June 13, 2018 12:59
@mfrey mfrey changed the title removes *.c includes removes source file includes in ccnl-utils Jun 13, 2018
@mfrey
Copy link
Collaborator Author

mfrey commented Jun 13, 2018

For the sake of transparency - I've missed to split ccnl-ext-hmac.c into ccnl-ext-hmac.{h,c}. This would require to remove the USE_HMAC256 use flag (like I've did for USE_SIGNATURES) since ISO C forbids empty translation unit. Before I do so, I would leave this up for discussion and get some feedback if that's the smartest choice.

@mfrey
Copy link
Collaborator Author

mfrey commented Jun 13, 2018

Before I do so, I would leave this up for discussion and get some feedback if that's the smartest choice.

Okay. I've seperated the source into a header and source of ccnl-ext-hmac.c and lib-sha256.c - There should be no issue with empty translation units.

blacksheeep
blacksheeep previously approved these changes Jun 13, 2018
Copy link
Contributor

@blacksheeep blacksheeep left a comment

Choose a reason for hiding this comment

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

Looks good to me.
Minor point: add documentation to hmac? would this be useful?

@mfrey
Copy link
Collaborator Author

mfrey commented Jun 13, 2018

Minor point: add documentation to hmac? would this be useful?

Will do and update the PR

@mfrey mfrey force-pushed the remove_c_includes branch from c7d7d68 to 2d47ca5 Compare June 14, 2018 07:46
@mfrey
Copy link
Collaborator Author

mfrey commented Jun 14, 2018

Minor point: add documentation to hmac? would this be useful?

I've tried my best, but there are some gaps to fill. Particularly, the offset parameter in the packet crafting functions of ccnl-ext-mac.c is hard to understand (given the fact that there are so many other offset/position parameters).

@mfrey mfrey merged commit a72c2c8 into cn-uofbasel:master Jun 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor requires squashing commits need to be squashed before merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants