Re: [PATCH] Cleanup: snprintf() always NUL-terminates: depend on it

From: Giel van Schijndel
Date: Sun Jan 04 2015 - 17:48:14 EST


On Sun, Jan 04, 2015 at 11:34:43 -0800, Linus Torvalds wrote:
> On Sun, Jan 4, 2015 at 10:05 AM, Giel van Schijndel <me@xxxxxxxxx> wrote:
>> Especially since one very strange piece of code seems to be written in
>> such a way that a NUL needs to be placed where a NUL is present already.
>
> Actually, it's worse than that. This:
>
>> len = snprintf(fname, 99, "%s", buf);
>> - fname[len-1] = '\0';
>
> is complete garbage, since the return value of snprintf() is not the
> length of the result, but length of what the result *would* have been.
>
> So if the string doesn't fit in 99 bytes, it will actively corrupt
> some random memory after the string. It's not writing zero to what was
> already zero, it's corrupting memory.

Ah yes, I didn't even notice that nasty side effect. I just deleted that
"really, really" NUL-termination line because it was based on a
misunderstanding of snprintf()'s postcondition. Even if
len==sizeof(fname) this still would have given the wrong example for
others to follow.

> Anyway, from a quick glance your patches look fine, but you need to
> sign off on them. See Documentation/SubmittingPatches.

Ah yes, forgot that. Would it be sufficient if I sent a reply to all
those patch mails with next line tacked on, or would it require a
resubmission?
> Signed-off-by: Giel van Schijndel <me@xxxxxxxxx>

--
Met vriendelijke groet,
With kind regards,
Giel van Schijndel
--
"Always code as if the guy who ends up maintaining your code will be a
violent psychopath who knows where you live."
-- Rick Osborne

Attachment: signature.asc
Description: Digital signature