search

sphincs / parallel-sphincsplus

This repository contains another implementation of the Sphincs+ postquantum signature system
Creative Commons Zero v1.0 Universal
3 stars 4 forks source link

Updated organization, makefile, and dependencies #18

Closed Ellie-42 closed 3 years ago

Ellie-42 commented 3 years ago

Issue 1) Organization. The files were simply not. This has been fixed for the main files by separating the source and the header files. This helps with issue 2

Issue 2) Makefile. The makefile had a few issues. It was only designed to work for the tests target, but not much else. The library itself did not work standalone, I learned very quickly. There were dependency issues in several files, <immintrin.h> was "immintrin.h" which has been known to cause issue in some compilers. <stdexcept> was missing in a file where std::runtime_error was called, and so forth. These have all been fixed, and now a functional library archive is built when the target install is run. It also default to /usr/bin/g++ and I commnetedthese variables as this is not a global compiler setting, leting make choose where, or the end user decide where.

Issue 3) Running make by default built the tests. I fixed this to build the objects instead. To build the tests, run make make install and then make tests This helps to better spot compilation, archival, and runtime errors, in order

Issue 4) The clean target removed everything that wasn't source from the directory and didn't clean t the sub-directories. It now clean the main directory, the subdirectories, and removes the archive in /lib There are two new targets to reflect this. cleanall is just clean with a more specific name. cleanlib removes the library files itself to permit for reinstall of the library if needed to be rebuilt.

sfluhrer commented 3 years ago

I should emphasize; I have no problem with you reorgnizing the files and the Makefile (which may not be the best possible); I'm just wondering why your rng changes are actually an improvement. BTW: I've been busy trying to add doxygen comments, which is something I need to do...

Ellie-42 commented 3 years ago

I should emphasize; I have no problem with you reorganizing the files and the Makefile (which may not be the best possible); I'm just wondering why your rng changes are actually an improvement. BTW: I've been busy trying to add doxygen comments, which is something I need to do...

Yeah Sorry, Just have other stuff to deal with. I changed it because I had nothing but errors when i tried to follow the semantics that were done as it stands. However, perhaps this could be resolved with an example, perhaps.

The issue I kept facing is that it expects everything to be const. This does not work when trying to use a third party random generator. Though, maybe instead of rewriting the semantics completely, there could be overloaded functions instead. One that takes the current random source, and one that uses an alternative source (providing it inherits from the same class)

I can write up the code in the coming days to accommodate this. Especially because for some, it may also be easier to do one way versus another. To me, it makes sense to provide as many options as possible for developers to utilize. A little more work for a little more convenience. I'll make this change in the coming days and open a new request.

Unless the const identifier is unnecessary, in which case dropping it than overloading it would be trivial;.

The makefile is definitely not the best possible, but it is a tad better.

As for doxygen, I'll adjust my changes to the doxygen documented ones and see what else I can do in that vein.