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.
