[BUG] umount mountpoint completion

[BUG] umount mountpoint completion

[Ferdinand Thiessen]

Hello,

I noticed following bug since at least zsh 5.3:

> % cd /tmp
> % mkdir "a 0"
> % sudo mount -o bind /opt "a 0"
> % sudo umount "/tmp/a *TAB*

*TAB* is the position when tabulator key is pressed.
This results in 5.3 to this:

> % sudo umount "/tmp/a\ 0"

Which is of course invalid. And with 5.3.1 it results to this:

> % sudo umount "/tmp/a^@"

The hex value of the result is: 2f74 6d70 2f61 00.

Regards,

Ferdinand

Re: [BUG] umount mountpoint completion

[Ferdinand Thiessen]

Am 05.03.2017 um 16:08 schrieb Ferdinand Thiessen:
> Hello,
>
> I noticed following bug since at least zsh 5.3:

There was a failure: When I wrote 5.3 I meant 5.0.5.
But 5.3.1 meant 5.3.1 ;-)

>
>> % cd /tmp
>> % mkdir "a 0"
>> % sudo mount -o bind /opt "a 0"
>> % sudo umount "/tmp/a *TAB*
> *TAB* is the position when tabulator key is pressed.
> This results in 5.3 to this:
>
>> % sudo umount "/tmp/a\ 0"
> Which is of course invalid. And with 5.3.1 it results to this:
>
>> % sudo umount "/tmp/a^@"
> The hex value of the result is: 2f74 6d70 2f61 00.
>
> Regards,
>
> Ferdinand
>

Re: [BUG] umount mountpoint completion

[Bart Schaefer]

On Mar 5,  4:08pm, Ferdinand Thiessen wrote:
}
} > % sudo umount "/tmp/a *TAB*
} > % sudo umount "/tmp/a^@"

Would you please replace TAB with ^X? (ctrl-x question-mark) to invoke
the _complete_debug widget, and make the trace file that is created
available somewhere?

Re: [BUG] umount mountpoint completion

[Ferdinand Thiessen]

Am 06.03.2017 um 05:01 schrieb Bart Schaefer:
> On Mar 5,  4:08pm, Ferdinand Thiessen wrote:
> }
> } > % sudo umount "/tmp/a *TAB*
> } > % sudo umount "/tmp/a^@"
>
> Would you please replace TAB with ^X? (ctrl-x question-mark) to invoke
> the _complete_debug widget, and make the trace file that is created
> available somewhere?
>
For 5.0.5 it should be this one:
https://wwwpub.zih.tu-dresden.de/~s9416139/zsh_report/zsh_5_0_5.log

And for 5.3.1 it should bis this:
https://wwwpub.zih.tu-dresden.de/~s9416139/zsh_report/zsh_5_3_1.log

Regards,
Ferdinand

Re: [BUG] umount mountpoint completion

[Bart Schaefer]

So ... there's a comment in the old _mount discussing what goes on here,
which was removed (and the code partially broken) by workers/33963.

The comment used to say:

-  # "Well, dear, the clever people who wrote Linux decided that some
-  # funny characters that might confuse programmes looking at the names
-  # would be encoded as octal escapes, like for example \040 for space.
-  # The clever people who wrote zsh decided that nothing would
-  # ever be quite as simple as it should be, so to substitute octal
-  # escapes everywhere in a string, even though the shell understands
-  # them natively in print escapes, needs some hackery where you match
-  # the octal number using the numeric closure syntax introduced after
-  # 4.3.4, then reinput the number in a standard math mode format as 8#OOO,
-  # and turn that into a character using the (#) parameter flag."

The fancy substitution that cleaned up this mess was then replaced by a
simpler one that works NEARLY all the time; except it breaks when the
aforementioned \040 for space is followed by ANOTHER digit.  In that
case the simplfied substitution treats (in Ferdinand's example) \0400
as a single octal number and turns it into $'\C-@' instead of " 0".

So perhaps 33963 needs to be reverted, and something less flippant
added to the comment to explain that the reason for bouncing back and
forth between numeric closure syntax and math mode format is to be sure
that EXACTLY THREE characters are interpreted as an octal number here.

However, one question remains.  Ferdinand, in your original message on
this thread you said:

} > % sudo umount "/tmp/a\ 0"
} 
} Which is of course invalid.

Could you please explain why you consider this to be invalid?  Because if
all we do is revert 33963, the above is what you're going to get again,
and I don't immediately see what's wrong with it.

Re: [BUG] umount mountpoint completion

[Daniel Shahaf]

Bart Schaefer wrote on Mon, Mar 06, 2017 at 15:10:30 -0800:
> The fancy substitution that cleaned up this mess was then replaced by a
> simpler one that works NEARLY all the time; except it breaks when the
> aforementioned \040 for space is followed by ANOTHER digit.  In that
> case the simplfied substitution treats (in Ferdinand's example) \0400
> as a single octal number and turns it into $'\C-@' instead of " 0".

Good find.

> However, one question remains.  Ferdinand, in your original message on
> this thread you said:
> 
> } > % sudo umount "/tmp/a\ 0"
> } 
> } Which is of course invalid.
> 
> Could you please explain why you consider this to be invalid?  Because if
> all we do is revert 33963, the above is what you're going to get again,
> and I don't immediately see what's wrong with it.

umount's argv would include the backslash literally, and there is no
mount point called '/tmp/a\ 0', only '/tmp/a 0'.

Re: [BUG] umount mountpoint completion

[Bart Schaefer]

On Mar 7, 11:19am, Daniel Shahaf wrote:
} Subject: Re: [BUG] umount mountpoint completion
}
} > } > % sudo umount "/tmp/a\ 0"
} > } 
} > } Which is of course invalid.
} > 
} > Could you please explain why you consider this to be invalid?
} 
} umount's argv would include the backslash literally, and there is no
} mount point called '/tmp/a\ 0', only '/tmp/a 0'.

If this is just completed as
    % umount /tmp/a<TAB>
is the result the same, or is the extra quoting happening because of
_normal being called by _arguments from _sudo?  I'm having a hard
time tracing down that bit.

Re: [BUG] umount mountpoint completion

[Ferdinand Thiessen]

Am 07.03.2017 um 20:14 schrieb Bart Schaefer:
> If this is just completed as
>     % umount /tmp/a<TAB>
> is the result the same, or is the extra quoting happening because of
> _normal being called by _arguments from _sudo?  I'm having a hard
> time tracing down that bit.

No if you use it without quotes it works:

% umount /tmp/a<TAB>

https://wwwpub.zih.tu-dresden.de/~s9416139/zsh_report/zsh-5.0.5-no-quotes.log

is completed to:

% umount /tmp/a\ 0

https://wwwpub.zih.tu-dresden.de/~s9416139/zsh_report/zsh-5.0.5-quoted.log

Re: [BUG] umount mountpoint completion

[Bart Schaefer]

On Mar 7,  9:37pm, Ferdinand Thiessen wrote:
} Subject: Re: [BUG] umount mountpoint completion
}
} Am 07.03.2017 um 20:14 schrieb Bart Schaefer:
} > If this is just completed as
} >     % umount /tmp/a<TAB>
} > is the result the same, or is the extra quoting happening because of
} > _normal being called by _arguments from _sudo?  I'm having a hard
} > time tracing down that bit.
} 
} No if you use it without quotes it works:
} 
} % umount /tmp/a<TAB>

Heh, that's helpful but it's not exactly what I meant to ask.

With sudo and quotes -> broken
With sudo without quotes -> ??  (what I didn't think to ask)
With quotes without sudo -> ??  (what I intended to ask)
With neither quotes nor sudo -> works  (what you told me)

Those two most recent traces you linked are exactly the same except for
some lines where the values ofthe variables that track quoting state
are visible, but make no change in the execution.

Re: [BUG] umount mountpoint completion

[Ferdinand Thiessen]

Am 07.03.2017 um 22:49 schrieb Bart Schaefer:
> Heh, that's helpful but it's not exactly what I meant to ask.
>
> With sudo and quotes -> broken
> With sudo without quotes -> ??  (what I didn't think to ask)
works
> With quotes without sudo -> ??  (what I intended to ask)
does not work (same as with sudo)
> With neither quotes nor sudo -> works  (what you told me)

I hope I got it this time right ;-)
If you need more information I will try to provide it.
Regards,
Ferdinand

Re: [BUG] umount mountpoint completion

[Bart Schaefer]

OK, Ferdinand, let's try the following patch.

There might still be the bug reported in 40761 by Philipp G. Haselwarter,
but if so I think that's down to _canonical_paths and not _mount.

diff --git a/Completion/Unix/Command/_mount b/Completion/Unix/Command/_mount
index 9ab279d..9a7041d 100644
--- a/Completion/Unix/Command/_mount
+++ b/Completion/Unix/Command/_mount
@@ -959,9 +959,15 @@ udevordir)
   esac
 
   local MATCH MBEGIN MEND
-  mp_tmp=("${(@g::)mp_tmp}")
-  dpath_tmp=( "${(@Mg::)dev_tmp:#/*}" )
-  dev_tmp=( "${(@g::)dev_tmp:#/*}" )
+  # The complicated substitution for mount point names is required because
+  # characters in /etc/mtab that might confuse programs reading the names
+  # are encoded as exactly 3 octal digits, like for example \040 for space.
+  # The cleaner-looking ${(g::)mp_tmp} might consume too many digits.
+  # Both mp_tmp and dev_tmp are derived from /etc/mtab or "mount" output.
+  mp_tmp=("${(@)mp_tmp//(#m)\\[0-7](#c3)/${(#)$(( 8#${MATCH[2,-1]} ))}}")
+  dev_tmp=("${(@)dev_tmp//(#m)\\[0-7](#c3)/${(#)$(( 8#${MATCH[2,-1]} ))}}")
+  dpath_tmp=( "${(@M)dev_tmp:#/*}" )
+  dev_tmp=( "${(@)dev_tmp:#/*}" )
 
   _alternative \
     'device-labels:device label:compadd -a dev_tmp' \