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.