Richard Guenther wrote:
> On Thu, Apr 19, 2012 at 12:42 PM, Jim Meyering <[email protected]> wrote:
>> Found by inspection.
>> Seeing this strncpy use without the usual following NUL-termination,
>> my reflex was that it was a buffer overrun candidate, but then I
>> realized this is gcc, so that's not very likely.
>> Looking a little harder, I saw the preceding strlen >= sizeof buf test,
>> which means there is no risk of overrun.
>>
>> That preceding test means the use of strncpy is misleading, since
>> with that there is no risk of the name being too long for the buffer,
>> and hence no reason to use strncpy.
>> One way to make the code clearer is to use strcpy, as done below.
>> An alternative is to use memcpy (buf, m->name, strlen (m->name) + 1).
Thanks for the quick feedback.
> Without a comment before the strcpy this isn't any more clear than
Good point.
How about with this folded in?
diff --git a/gcc/genmodes.c b/gcc/genmodes.c
index f786258..3833017 100644
--- a/gcc/genmodes.c
+++ b/gcc/genmodes.c
@@ -452,6 +452,7 @@ make_complex_modes (enum mode_class cl,
if (cl == MODE_FLOAT)
{
char *p, *q = 0;
+ /* We verified above that m->name+NUL fits in buf. */
strcpy (buf, m->name);
p = strchr (buf, 'F');
if (p == 0)
> when using strncpy. Also your issue with the terminating NUL is
> still present for the snprintf call done later (in fact the code looks like
> it can be optimized, avoiding the strcpy in the path to the snprintf).
Isn't it different with snprintf?
While strncpy does *NOT* necessarily NUL-terminate,
snprintf explicitly does, always.
IMHO, if you'd like to avoid the duplication in the strcpy/snprintf
path, that deserves to be a different change.
