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

[Jim Meyering]

Pádraig Brady wrote:
> 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

Thanks for all the speedy feedback!