Re: PATCH: Fix _file_descriptors

[Mikael Magnusson <mikachu@xxxxxxxxx>]

On 27 February 2012 17:08, Oliver Kiddle <okiddle@xxxxxxxxxxx> wrote:
> --- On Sun, 26/2/12, Mikael Magnusson <mikachu@xxxxxxxxx> wrote:
>
>> I noticed file descriptor completion
>> didn't work, with the verbose style set
>> because when this is run,
>>   fds=( /dev/fd/<0-9>(N:t) )
>> the /dev/fd dir is open while the glob is performed, which
>> results in
>> a spurious entry in the result, which then cannot be
>> dereferenced. The
>> result is that the list array is not aligned to the fds
>> array (and also
>> an error message is output), and an fd that doesn't exist is
>> completed.
>
> I'm not sure what you mean by "cannot be dereferenced".

The link does not exist, so you cannot lstat it.

> In any case, this whole function can be simplified by using the newish :A modifier.

:A doesn't do anything useful for fds that point to things like
pipe:[21325192] or 'file (deleted)'. (nor does (:a)).
% echo /proc/$$/fd/2(:A); zstat +link /proc/$$/fd/2
/proc/3555/fd/2
/tmp/foo/test (deleted)

% echo /proc/$$/fd/3(:A); zstat +link /proc/$$/fd/3
/proc/3555/fd/3
socket:[219517545]

(We could also compare with ztcp -L before checking the symlinks, etc.)

> Also, I'm fairly certain that it is intentional that this function only completes file descriptors from 0 - 9.

I'm not, the code was originally a loop over {0..9} and probably the 9
was just some arbitrary low number. What reason could there possibly
be to do this?

> With :A, assigning fds can just be:
>
> fds=( /dev/fd/<0-9>(e,'[[ $REPLY:A != /proc/$$/fd ]]',) )
>
> It's probably best to build up list then with a for loop after checking the style but again :A can be used instead of trying the three old mechanisms.

As I noted originally, the problem with this is that there will be one
link in $fds that doesn't exist when you try to build up the list
array.

> Another approach would be to use a subshell to open the directory:
>  fds=( $(print /proc/$$/fd/<0-9>(N:t)) )
> But I think there are some platforms that have /dev/fd but not /proc/$$/fd.

These should still work, as the fallback still uses /dev/fd. I guess
we could try both for the verbose style too though.

-- 
Mikael Magnusson