Re: [PATCH] Improved error messages when temporary file creation fails

[Jakub Narebski <jnareb@gmail.com>]

Arnout Engelen wrote:
> On Tue, Dec 07, 2010 at 12:56:17PM -0800, Junio C Hamano wrote:

>>> +   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