Re: [PATCH] truncate: don't leak a file descriptor with --ref=PIPE

[Pádraig Brady]

On 08/04/2012 11:26 AM, Jim Meyering wrote:
> Pádraig Brady wrote:
>> On 08/04/2012 10:06 AM, Jim Meyering wrote:
> ...
>>> Subject: [PATCH] truncate: don't leak a file descriptor with --ref=PIPE
>>>
>>> * src/truncate.c (main): For a user who makes the mistake of
>>> using a non-seekable file as a reference for the desired length,
>>> truncate would open that file, attempt to seek to its end, but
>>> upon seek failure would neglect to close the file descriptor.
>>> Reverse conjuncts, so the close is unconditional.
>>> Spotted by coverity.
>>
>> Cool.
>> Not worth a news entry because we exit() right after
>> (as file_size will be left at -1 in this case).
>>
>> Hmm, but I just noticed that close() may clear
>> the errno of an lseek() failure, so we'd get
>> the classic "Error: success" in this case?
> 
> Good catch.
> That close call *could* technically clobber errno.
> For the record, on linux it does not clear it upon success:
> 
>     $ mkfifo F && ./truncate --ref=F k & echo > F
>     ./truncate: cannot get the size of 'F': Illegal seek
> 
> Hmm...
> do we really care if the close fails for a read-only FD
> for which our lseek (the important call) succeeded?
> 
> I don't think so.
> How about this improved patch:

+1

cheers,
Pádraig.