ltc_ecc_mul2add.c integer sign issue

[karel-m]

I got the following compiler warning:

"src/ltc/pk/ecc/ltc_ecc_mul2add.c", line 137: warning #2068-D: integer
          conversion resulted in a change of sign
    for (x = -1;; ) {

The code in question:

043  unsigned       bitbufA, bitbufB, lenA, lenB, len, x, y, nA, nB, nibble;
...
136  /* for every byte of the multiplicands */
137  for (x = -1;; ) {
138     /* grab a nibble */
139     if (++nibble == 4) {
140        ++x; if (x == len) break;
141        bitbufA = tA[x];
142        bitbufB = tB[x];
143        nibble  = 0;
144     }

It seems that we are assigning -1 to unsigned integer.

I have not investigated yet what would be the best fix.

--Karel

[sjaeckel]

well, very stupid idea but have you tried for (x = 0-1;; )? :-)

[karel-m]

Looking at the code - https://github.com/libtom/libtomcrypt/blob/develop/src/pk/ecc/ltc_ecc_mul2add.c#L37 - it seems that changing the type of x from unsigned to int might be better.

Something like this (not tested yet):

  ecc_point     *precomp[16];
  unsigned       bitbufA, bitbufB, lenA, lenB, len, nA, nB, nibble;
  int            x, y;
  unsigned char *tA, *tB;

Note: y is changed just for consistency as both x and y are used for indices.

[sjaeckel]

Or should we probably rewrite the implementation to be more clear?
this looks like very complicated but I suppose there would be a cleaner way to write this

137  for (x = -1;; ) {
138     /* grab a nibble */
139     if (++nibble == 4) {
140        ++x; if (x == len) break;

[karel-m]

I have tested that the following works without warnings:

  ecc_point     *precomp[16];
  unsigned       bitbufA, bitbufB, lenA, lenB, len, nA, nB, nibble;
  int            x, y;
  unsigned char *tA, *tB;

Should I send a PR or do we want to change the implementation?

[sjaeckel]

I'm fine with the fix