[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: dateutils: FTBFS on hurd-i386 (for review)



Svante,

On 15/05/14 22:39, Svante Signell wrote:
> On Thu, 2014-05-15 at 13:38 +0200, Philipp A. Hartmann wrote:
> 
> Yes, you are right. However, setting len to zero would have solved that.

I'm not sure if passing len=0 to snprintf is portable in all cases.  If
you insist on reporting the error, I'd prefer to use "error()" and avoid
the snprintf call by early returning NULL.

>>>  #if !defined L_tmpnam
>>>  # define L_tmpnam	(PATH_MAX)
>>>  #endif	/* !L_tmpnam */
>>> -	static char expfn[PATH_MAX];
>>> -	static char actfn[PATH_MAX];
>>
>> Why not simply replace PATH_MAX with L_tmpnam and be done with
>> everything?  Is this defined on GNU/Hurd?  Is it used elsewhere in the file?
> 
> No and no.

If this is currently not used, it should probably be dropped to avoid a
later reintroduction of PATH_MAX.

>>> +	static char *expfn = NULL;
>>> +	static char *actfn = NULL;
>>
>> Why keep them static?
> 
> Why not?

Because static variables are shared across multiple invocations of the
function.  In the case at hand, this leads to a skipped
(re)initialization to NULL from the second call onwards and the memory
handling breaks.

> +		if ( expfn == NULL || mkfifo(expfn, 0666) < 0) {

Whitespace error.
		if (expfn ...

> +	if ( actfn == NULL || mkfifo(actfn, 0666) < 0) {

Same here.

/Philipp


Reply to: