Re: [PATCH] Improved error messages when temporary file creation fails
> On Tue, Dec 07, 2010 at 12:56:17PM -0800, Junio C Hamano wrote:
Show 28 quoted lines
>>> + char origtemplate[255];
>>> + strlcpy(origtemplate, template, 255);
>>
>> Why "255"?
>
> Random - 'i had to choose something'.
>
>> It may happen to be sufficiently large for the current callers, but what
>> provisions if any are made to help the compiler or the runtime protect us
>> from new and broken callers? Use of strlcpy() there hides the issue from
>> the runtime by avoiding segfault, but it actively harms us by making the
>> code silently behave incorrectly without segfaulting, no?
>
> Only in a small way: when a bigger template is encountered and the mkstemp
> call succeeds, there is no problem. Only when xmkstemp fails *and* clears the
> template, the diagnostic error message shows a truncated version of the
> original.
>
> I *could* dynamically allocate space for the original template string, but that
> would mean I'd need to do a malloc() instead of putting the buffer on the stack
> like this, and free() it afterwards. I'm not too concerned about the
> performance hit here (presumably the I/O that comes with creating and using
> the temporary file here is orders of magnitude slower than that malloc()
> anyway), but it would also make the code a bit less easy to read.
>
> What do you think would be preferable here, a simple fixed-length buffer on the
> stack that might cause a truncated error message or a dynamically-allocated
> one that makes the code somewhat more complicated?
Why not use PATH_MAX instead of 255?
P.S. I'm sory for cutting up CC list...
--
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git