liteserver / binn

Binary Serialization
Apache License 2.0
440 stars 58 forks source link

aes.hpp superfluous - C++/C has standards support semantics not reflected here. #19

Closed ascheurer closed 6 years ago

ascheurer commented 6 years ago

I've seen a lot of changes over the last few months and many positive - increased attention to FIPS compliance which is very important to my intended efforts w/ AES256 standards compliance/interoperability with FPGA/PL implementations of AES256 standard and other implementations - very important to adhere to standards.

However the use of aes.hpp does not follow a standard support of C++ and C usages.

Currently aes.hpp contains:

ifndef _AESHPP

define _AESHPP

ifndef __cplusplus

error Do not include the hpp header in a c project!

endif //__cplusplus

extern "C" {

include "aes.h"

}

endif //_AESHPP

and no issue with using aes.hpp to imply C++ as boost does where .hpp means C++ and this is fine. However this is used for C++ code that includes C++ keywords like class, private, etc... basically class definitions that if used in a .c module would look out of place - its .hpp so it goes with .cpp

However aes.hpp is not correct usage.

aes.h needs to have function prototypes only - not preprocessor symbols or structs - these are common in C and C++ but only function prototypes to use the __cplusplus reserved pre-processor symbol for C++ but not recognized in C.

Furthermore using prepended and post underscores is not necessary and does not comply w/ the standard as the use of _ with preprocessor symbols is reserved by the compiler - eg. TIME but also other symbols

ifndef _AESH

define _AESH

needs only be AES_H

aes.h to be compliant with C/C++ interoperability standards (ANSI C and C99) would be

ifndef AES_H

define AES_H

...

ifdef __cplusplus

extern "C" {

endif

/ function prototypes only ... / void AES_ECB_encrypt(struct AES_ctx ctx, const uint8_t buf); ...

ifdef __cplusplus

}

endif

Now this is usable by C and C++ transparently - aes.hpp does not need to be used at all unless you do want to create a C++ wrapper in aes.hpp and that would make sense for those C++ folks that want a class interface w/ private data encapsulating context etc...

its significantly better to support transparent usage than issue an #error from the preprocessor when a C module includes a .hpp file ... if the .hpp includes a class keyword it will be flagged as an error in any case.

One could also an error in the .hpp as you've done if a C module includes it ... however it shouldn't be necessary for a C++ program to use a .hpp file if they only want the C functions to be extern "C" -- compilers do this with all the standard C library functions like strcpy, memcpy etc... where a C++ program can include stdlib.h without having to use or any other non-C++ include - C includes must use the above method to be usable transparently by C and C++ programs.

aes.h as it currently standards is not compliant to standard C and C++ usage.

ascheurer commented 6 years ago

entered comment for another git hub package - sorry for mistake.