On 06/30/2012 03:11 PM, Pádraig Brady wrote: > On 06/30/2012 12:53 PM, Paul Eggert wrote: >> On 06/29/2012 07:55 AM, Pádraig Brady wrote: >>> Also the in==out case, you'd like to check for write-ability too. >>> >>> Both cases could be handled I think with something like: >>> >>> if (access (outfile, W_OK) != 0 && errno != ENOENT) >>> error (...); >> >> Wouldn't it be better to actually open the output file, >> but not truncate it? We can then truncate it just before >> actually writing to the file. That would avoid a race >> condition or two. >> >> In the in==out case, we could tune this by opening >> the file just once, with O_RDWR. If the file is not >> a regular file, we might have to give up and open such >> a file twice, but that should be rare. >> > > The race would be unlikely and > only fallback to the existing operation > of slower failure. > > Though I suppose opening the file is a > more direct check and would also obviate the > need to check for writeability of the containing dir > in the case of a non existent file. > > OK I'm leaning towards an early open so. > > As for cleaning up an empty created file, > `sort` already has an exit_cleanup() function, > so we can unlink there. > > I'm not sure it's worth tuning the in==out case TBH. So I didn't bother unlinking created empty files as this is problematic in the presence of symlinks. To mitigate this I create the output after all option validation is done, just before sort/merge process is started. Also we must be careful to handle the `sort -o missing missing` case. I.E. we don't want to create an empty file, resulting in the above failing to notice the missing file and returning succesfully. So to avoid that I explicitly check all inputs are readable first. In addition to catering for the above case, it's a general improvement to avoid redundant processing. That was already handled in the merge case, but in the sorting case only a stat was done as a side effect of input size checking, and that didn't handle the case where input was present but unreadable. Patch attached. cheers, Pádraig.
sort-exit-early.diff
Description: Text document
--- End Message ---