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!
