Tim Ruehsen tim.ruehsen@gmx.de writes:
In cast128.c I removed the wiping of t, l and r. Instead I set t=0 at the beginning of the loops (It seemed to be used uninitialized in F1 macro).
I don't think the wiping has any meaning. Perhaps it was intended to somehow avoid leaking intermediate values. (I'm not the original author of this file).
I don't understand the comment on t being used before initialized. As far as I see, all of F1, F2 and F3 first assign t, then apply some sbox substitutions on it. Am I missing something?
If you are addressing some compiler warnings, please, *pretty please*, quote them accurately.
I do get several warnings (false positives) on a different function in this file when I compile with gcc:
cast128.c: In function `nettle_cast128_set_key': cast128.c:276:1: warning: `t[3]' may be used uninitialized in this function [-Wmaybe-uninitialized] cast128.c:210:26: warning: `z[3]' may be used uninitialized in this function [-Wmaybe-uninitialized]
and a bunch more similar ones. I think I'd prefer to reorganize this function completely, I find the current organization hard to understand.
diff --git a/examples/eratosthenes.c b/examples/eratosthenes.c index 2b9d04f..0eea941 100644 --- a/examples/eratosthenes.c +++ b/examples/eratosthenes.c @@ -399,5 +399,10 @@ main (int argc, char **argv) printf("%lu\n", n); } }
- /* silence static analyzers */
- free(block);
- free(sieve);
- return EXIT_SUCCESS;
}
Why this? It ought to be the job of the C library to free all malloced storage after main returns. Do you get a warning for this? That sounds overly noisy to me.
diff --git a/examples/io.c b/examples/io.c index 7b2289c..cda9ed3 100644 --- a/examples/io.c +++ b/examples/io.c @@ -125,7 +125,10 @@ read_file(const char *name, unsigned max_size, char **contents) fclose(f);
/* NUL-terminate the data. */
- buffer[done] = '\0';
- if (buffer)
- buffer[done] = '\0';
- else
- done = 0; *contents = buffer;
This ought not to happen. Hmm. I think the only way the execution can omit the realloc call (and the check for NULL return value) in the loop is if feof returns true immediately after fopen. Does you analyzer agree with that?
Then the correct fix is to move the feof call to after fread, as suggested in the preceding FIXME comment.
diff --git a/examples/nettle-benchmark.c b/examples/nettle-benchmark.c index b76a91c..48e53d0 100644 --- a/examples/nettle-benchmark.c +++ b/examples/nettle-benchmark.c @@ -96,7 +96,7 @@ static double frequency = 0.0; #define BENCH_ITERATIONS 10 #endif
-static void +static void NORETURN die(const char *format, ...) { va_list args;
Applied.
diff --git a/tools/misc.h b/tools/misc.h index 70c9eeb..af55998 100644 --- a/tools/misc.h +++ b/tools/misc.h @@ -28,19 +28,10 @@ #endif
void -die(const char *format, ...) -#if __GNUC___
__attribute__((__format__ (__printf__,1, 2)))__attribute__((__noreturn__))-#endif
;+die(const char *format, ...) PRINTF_STYLE(1,2) NORETURN;
void -werror(const char *format, ...) -#if __GNUC___
__attribute__((__format__ (__printf__,1, 2)))-#endif
;+werror(const char *format, ...) PRINTF_STYLE(1,2);
void * xalloc(size_t size);
Applied. I found one more werror prototype, in examples/io.h, which I improved in the same way (maybe that was in another patch of yours? I haven't reviewed all yet).
Regards, /Niels