jschanck / ntru

Implementations of the NIST post-quantum cryptography process finalist NTRU.
https://ntru.org
Creative Commons Zero v1.0 Universal
41 stars 8 forks source link

Comparison between signed and unsigned #12

Closed mouse07410 closed 4 years ago

mouse07410 commented 4 years ago

The current code contains comparison of unsigned long to int (signed aaginst unsigned). Compilers don't like it, and issue warnings. Here's a proposed fix (since buffer_pos is unlikely to be negative):

diff --git a/ref-common/rng.h b/ref-common/rng.h
index 577e263..1ae2154 100644
--- a/ref-common/rng.h
+++ b/ref-common/rng.h
@@ -17,7 +17,7 @@

 typedef struct {
     unsigned char   buffer[16];
-    int             buffer_pos;
+    unsigned long   buffer_pos;
     unsigned long   length_remaining;
     unsigned char   key[32];
     unsigned char   ctr[16];

Perhaps there are other places in the code where indices are defined as int? If so, could I suggest changing them to unsigned long or size_t?

jschanck commented 4 years ago

This file is from NIST and is only used in PQCgenKAT_kem.c. I'm not going to remove it or modify it since it's useful for testing.

FYI this repo is upstream to https://github.com/PQClean/PQClean, which is the preferred repo to branch from if you're putting NTRU into another library.

mouse07410 commented 4 years ago

I'm not going to remove it or modify it since it's useful for testing.

This file causes modern compiler to flag comparison between unsigned long and int. If you're OK with that - fine, you own this code. I'd be wary of comparisons like that.

Update

I checked https://github.com/PQClean/PQClean - am I correct that this repo does not contain optimized code?