JordanCheney commented 8 years ago

30 This is an update to use iostreams for io instead of serializing to char * arrays. Because it is a larger change I'm keeping it separate from the phase 2 branch until it can be reviewed. @stephenrawls @paddygr could ISI and NIST please review when you have a moment.

stephenrawls commented 8 years ago

Hi @JordanCheney,

In general I like the idea of using iostreams here, although I will note that it does have a few implications. I believe that prior to this point the API was pure C, whereas now we are exposing C++ elements into the API layer. Due to historic design decisions with c++ (e.g. no standardization in name mangling; no ABI compatibility with different C++ standard libraries), I think this will cause integration issues. In particular, I believe it will force implementors (e.g. ISI) to compile their code with the same c++ compiler as client code (i.e. NIST). Weather or not that is considered a problem, it should at least be made explicit that this is a departure from the previous API.

Thanks, ~Stephen

JordanCheney commented 8 years ago

Hi Stephen,

The phase 2 API is a C++ API already (we use strings, vectors and operator overloading as well as iostreams now). I think that change was mentioned at the kickoff but I don't remember for sure. The issue of name mangling and ABI compatibility has not been brought up before. Since you are distributing source code directly to NIST for them to compile will this be an issue?

Best, Jordan

Noblis / INVSC-janus

Use iostreams for io instead of byte arrays #38

30 This is an update to use iostreams for io instead of serializing to char * arrays. Because it is a larger change I'm keeping it separate from the phase 2 branch until it can be reviewed. @stephenrawls @paddygr could ISI and NIST please review when you have a moment.