Re: [COMMITTED PATCH] Clean up POSIX.1 implementation of truncate.

["Carlos O'Donell" <carlos at redhat dot com>]

On 05/07/2013 07:10 AM, Rich Felker wrote:
> On Mon, May 06, 2013 at 02:57:03PM -0700, Roland McGrath wrote:
>> 2013-05-06  Roland McGrath  <roland@hack.frob.com>
>>
>> 	* sysdeps/posix/truncate.c (__truncate): Renamed from truncate.
>> 	Call __ names for open, ftruncate, and close.
>> 	For LENGTH==0 case, just use O_TRUNC rather than calling ftruncate.
>> 	(truncate): Define as weak alias.
>>
>> --- a/sysdeps/posix/truncate.c
>> +++ b/sysdeps/posix/truncate.c
>> @@ -1,4 +1,5 @@
>> -/* Copyright (C) 1995-2013 Free Software Foundation, Inc.
>> +/* Truncate a file given by name.  Generic POSIX.1 version.
>> +   Copyright (C) 1995-2013 Free Software Foundation, Inc.
>>     This file is part of the GNU C Library.
>>  
>>     The GNU C Library is free software; you can redistribute it and/or
>> @@ -22,20 +23,22 @@
>>  
>>  /* Truncate PATH to LENGTH bytes.  */
>>  int
>> -truncate (path, length)
>> -     const char *path;
>> -     off_t length;
>> +__truncate (const char *path, off_t length)
>>  {
>>    int fd, ret, save;
>>  
>> -  fd = open (path, O_WRONLY);
>> +  fd = __open (path, O_WRONLY | (length == 0 ? O_TRUNC : 0));
> 
> This code is buggy both before and after since it can leak a file
> descriptor if another thread or a signal handler performs exec while
> the file is open.
> 
> I understand this code is just the "generic POSIX implementation" in
> terms of other POSIX functions and not actually used anywhere, but I
> question the value of having buggy, nonconforming implementations like
> this.
> 
> Note that it can be fixed with O_CLOEXEC assuming the underlying
> implementation supports that.

I agree, if we can fix it with minimal effort then we should.

Cheers,
Carlos.