On Sun, 31 Jan 2016 12:39:34 +0100 Hiltjo Posthuma <hil...@codemadness.org> wrote:
> Some notes from me:
>
> - I would avoid using variadic arrays, use a sane subset
> of C99.
Are you refering to this code?
#define matrix (*matrix)
#define map (*map)
char map[an + 1][bn + 1] = emalloc(sizeof(char[an + 1][bn + 1]));
size_t matrix[2][bn + 1] = ecalloc(1, sizeof(size_t[2][bn + 1]));
Perhaps the #define:s makes it confusing, but I though the code
beneath it should be simplified by them and that it was worth it.
I was suggested to use
size_t (*matrix)[n][m] to simplify the matrix allocation.
Remove the #define:s and write (*) explicitly?
> - Use snprintf, instead of sprintf and the check.
Are you refering to the this code?
sprintf(buf + (sizeof("0000-00-00 00:00:00.") - 1), "%09lu",
attr->st_mtim.tv_nsec);
There are no checks, and the buffer is guaranteed
to be large enough, so I disagree about using snprintf.
> - Use strlcpy or snprintf instead of strcpy.
> - Avoid using the "inline" #defines.
I will look into these.
> - Disable colour output by default, maybe just add a flag
> for it to explicitly enable it.
Does it matter if stdout is a TTY? Anyway I rather remove
it than add another flag. But lets see what everyone thinks.
> - We should also allow using stdin for one of the file
> inputs (so can't "reread" file): diff file.c -
stdin (via -) should be supported.
>
> Kind regards,
> Hiltjo
>
pgp6rShRhxRnN.pgp
Description: OpenPGP digital signature
