Tim Ruehsen <[email protected]> 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
--
Niels Möller. PGP-encrypted email is preferred. Keyid C0B98E26.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list
[email protected]
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
