[9fans] Ugly hack

[9fans] Ugly hack

[Lucio De Re]

An ugly hack to Tom Duff's little jewel, rc(1).

I've found the ability to limit shell command line argument expansion
(only in English can one write something so succintly and so ugly) to
directories rather than all directory entries by suffixing the last
part with a "/".  This is normal behaviour in the NetBSD shells.

For example:

broken! lc
mkfile             rdesktop-1.1.0.tar trim
openldap-2.0.22    rx-1.5             trim.c
rdesktop-1.1.0     term.8             trim.out
broken! cd rdesk*
Usage: cd [directory]
broken! echo rdesk*
rdesktop-1.1.0 rdesktop-1.1.0.tar
broken! cd rdesk*/
broken! pwd
/usr/lucio/Projects/Sundry/rdesktop-1.1.0
broken!

Note how the trailing slash allows me to supply the cd command with
a single argument.

I have hacked plan9.c, glob.c and fns.h in /sys/src/cmd/rc to achive
this and I'd appreciate some feedback: is this useful to anyone
else?  Does it work properly (I have tested a few limit cases, but
I'm not sure Ive covered all possibilities)?

The patch is not vast, it largely required modifying the Readdir()
function to be rearranged and I'd appreciate comments on possible
improvements to my choice of coding.

All comments (not flames) appreciated.  I hope my sources for rc(1)
are not too far out of date.

term% diff plan9.c `{yesterday plan9.c}
321c321
< int Readdir(int f, char *p, int dironly)
---
> int Readdir(int f, char *p)
326,345c326,334
< 	for (;;) {
< 		if (dir[f].i==dir[f].n){	/* read */
< 			free(dir[f].dbuf);
< 			dir[f].i=0;
< 			dir[f].dbuf=0;
< 			if ((n=dirread(f, &dir[f].dbuf)) <= 0){
< 				dir[f].n=0;
< 				return 0;
< 			} else
< 				dir[f].n=n;
< 		}
< 		while(dir[f].i<dir[f].n){
< // print ("N: '%s(%d)'\n", dir[f].dbuf[dir[f].i].name,(dir[f].dbuf[dir[f].i].mode&0x80000000));
< 			if(!dironly || (dir[f].dbuf[dir[f].i].mode&0x80000000)){
< 				strcpy(p, dir[f].dbuf[dir[f].i].name);
< 				dir[f].i++;
< 				return 1;
< 			}
< 			dir[f].i++;
< 		}
---
> 	if(dir[f].i==dir[f].n){	/* read */
> 		free(dir[f].dbuf);
> 		dir[f].dbuf=0;
> 		n=dirread(f, &dir[f].dbuf);
> 		if(n>=0)
> 			dir[f].n=n;
> 		else
> 			dir[f].n=0;
> 		dir[f].i=0;
347c336,340
< 	return 0;
---
> 	if(dir[f].i==dir[f].n)
> 		return 0;
> 	strcpy(p, dir[f].dbuf[dir[f].i].name);
> 	dir[f].i++;
> 	return 1;

term% diff glob.c `{yesterday glob.c}
37c37
< void globdir(char *p, char *namep, int dironly)
---
> void globdir(char *p, char *namep)
64c64
< 	/* read the directory and recurse for any entry that matches */
---
> 	/* read the directory and recur for any entry that matches */
68,69c68
< // print ("D: %s\n", dironly?"yes":"no");
< 	while(Readdir(f, namep, dironly)){
---
> 	while(Readdir(f, namep)){
72c71
< 			globdir(newp, t, dironly);
---
> 			globdir(newp, t);
84d82
< 	int dironly;
93,99c91
< 	dironly=strlen(p)-1;
< // print ("G: '%s(%d)'\n", p, dironly);
< 	if (dironly>0 && p[dironly]=='/'){
< 		dironly=1;
< 	} else
< 		dironly=0;
< 	globdir(p, globname, dironly);
---
> 	globdir(p, globname);

term% diff fns.h `{yesterday fns.h}
16c16
< int	Readdir(int, char*, int);
---
> int	Readdir(int, char*);

Re: [9fans] Ugly hack

[Sam]

Would I were bothered by this, I'ld be more inclined
towards:

fn cdd { for(i) { if(test -d $i){ cd $i } } }

Note there's a bug in this approach due to rc's
for lacking a break equivalent, but it's unlikely
to bite.

Always better to use the tools than hack them if at all
possible.

Cheers,

Sam

Re: [9fans] Ugly hack

[Dan Cross]

This is the type of think I use walk and sor for all the time:

	walk -d 1 . | sor 'test -d'

Is basically the same as, ``echo */''.

	- Dan C.

Re: [9fans] Ugly hack

[Lucio De Re]

On Sat, Feb 15, 2003 at 11:18:41AM -0500, Sam wrote:
>
> Would I were bothered by this, I'ld be more inclined
> towards:
>
I get bitten by the presence of the tar file and extraction directory
in the same place, with unholily long names.  And, yes, RIO does help,
but I use too much Unix to wean myself off old habits.

> Always better to use the tools than hack them if at all
> possible.
>
It's (hopefully) a transparent adjustment.  And, even if possibly
undocumented, a step towards conventional Unix behaviour.  What I'm
curious about, is where it may not be as transparent as I think.

++L

Re: [9fans] Ugly hack

[Geoff Collyer]

	echo */.

would work on Unix, but on Plan 9 matches all files in the current directory
(fallout from use of cleanname, perhaps?).  However,

	echo */.. | sed 's;/\.\.($| ); ;g'

does print just the names of the directories in the current directory.

Re: [9fans] Ugly hack

[Lucio De Re]

On Sat, Feb 15, 2003 at 03:56:28PM -0800, Geoff Collyer wrote:
>
> 	echo */.
>
> would work on Unix, but on Plan 9 matches all files in the current directory
> (fallout from use of cleanname, perhaps?).  However,
>
A small fix to my hack:

	while(Readdir(f, namep, dironly||namep[1]!=0)){
				       ^^^^^^^^^^^^^

(adding a check for a trailing component) has the dual effect of
fixing the above inconsistency and, hopefully, speed up the expansion
by considering only directories if there is a subsequent component.

There may be implications I have overlooked.  I think the code should
be revised, if the objective seems worthwhile, rather than hacked, but
that's a decision for the _real_ developers.

++L

Re: [9fans] Ugly hack

[Lucio De Re]

On Sun, Feb 16, 2003 at 11:20:13AM +0200, Lucio De Re wrote:
> >
> A small fix to my hack:
>
> 	while(Readdir(f, namep, dironly||namep[1]!=0)){
> 				       ^^^^^^^^^^^^^
>
Oops!  Here's a clearer patch:

term% diff glob.c `{yesterday glob.c}
69c69
< 	while(Readdir(f, namep, dironly||namep[1]!=0)){
---
> 	while(Readdir(f, namep, dironly)){

++L

PS: I don't know how important it is to check whether there actually
is a speed improvement when expanding big directories.

Re: [9fans] Ugly hack

[Lucio De Re]

On Sun, Feb 16, 2003 at 11:25:27AM +0200, Lucio De Re wrote:
>
> Oops!  Here's a clearer patch:
>
> term% diff glob.c `{yesterday glob.c}
> 69c69
> < 	while(Readdir(f, namep, dironly||namep[1]!=0)){
> ---
> > 	while(Readdir(f, namep, dironly)){
>
And here's one that actually doesn't break globbing:

term% diff glob.c `{yesterday glob.c}
69c69
< 	while(Readdir(f, namep, dironly||*newp=='/')){
---
> 	while(Readdir(f, namep, dironly)){
term%

> PS: I don't know how important it is to check whether there actually
> is a speed improvement when expanding big directories.

Error discovered while trying to display /bin/*/* for timing
purposes.  I still don't know if there is a speed improvement.
But if the following is anything to go by:

term% time rc -c 'echo /bin/*/* > /tmp/out.2'
0.02u 0.31s 1.18r 	 rc -c echo /bin/*/* > /tmp/out.1
term% time /sys/src/cmd/rc/8.out -c 'echo /bin/*/* > /tmp/out.2'
0.03u 0.08s 0.27r 	 /sys/src/cmd/rc/8.out -c echo /bin/*/* > /tmp/out.2
term%

it looks pretty much a worthwhile improvement.

++L

Re: [9fans] Ugly hack

[Lucio De Re]

On Sun, Feb 16, 2003 at 02:18:59PM +0200, Lucio De Re wrote:
> >
> And here's one that actually doesn't break globbing:
>
> term% diff glob.c `{yesterday glob.c}
> 69c69
> < 	while(Readdir(f, namep, dironly||*newp=='/')){
> ---
> > 	while(Readdir(f, namep, dironly)){
> term%
>
OK, after this, I ought to shut up.  *newp=='/' suffices to determine
the value to be passed to the modified Readdir() function.  This
means that "dironly" within glob.c can be eliminated entirely.  The
following patch based on the original glob.c is much simpler:

term% diff glob.c /n/dump/2003/0215/sys/src/cmd/rc/glob.c
64c64
< 	/* read the directory and recurse for any entry that matches */
---
> 	/* read the directory and recur for any entry that matches */
68c68
< 	while(Readdir(f, namep, *newp=='/')){
---
> 	while(Readdir(f, namep)){
term%

Please forgive all the previous noise.

++L

Re: [9fans] Ugly hack

[Russ Cox]

This is not the right place for such a hack.
You are attempting to fix a bug in the kernel
by patching the shell.  Down that path lies
madness.  See http://plan9.bell-labs.com/sys/doc/lexnames.ps.

Russ

Re: [9fans] Ugly hack

[Lucio De Re]

On Sun, Feb 16, 2003 at 09:19:38AM -0500, Russ Cox wrote:
>
> This is not the right place for such a hack.
> You are attempting to fix a bug in the kernel
> by patching the shell.  Down that path lies
> madness.  See http://plan9.bell-labs.com/sys/doc/lexnames.ps.
>
Well, the shell does not make use of any special kernel features
to expand globbed arguments, it recursively descends the directory
tree.  I'm addressing that aspect and I'm quite pleased with the
results.

If the kernel were to provide an equivalent function, I'd certainly
be keen to see it incorporated in the shell and any other similar
command and would try to motivate for it to be as powerful and/or
practical as possible.

I did read Rob's paper a while back, but I don't recall it addressing
filename expansion.  I could be mis-remembering, of course.

In defense of my point of view, I must add the following: adding
the "dironly" filter to the shell code (in the Readdir() function
of plan9.c - I have an interesting challenge in duplicating this
in unix.c, ironically) not only makes rc's behaviour consistent
with my Unix-based expectations but also reduces the resources
required for expanding big and/or deep directories quite considerably.

I think it was echo /*/*/*/*/*/*/*/* that blew up with the release
shell and completed (apparently successfully, although one may
question the sanity of such an operation) with the modified shell.

It goes without saying that echo /*/*/*/*/*/*/*/*/ is not even
vaguely useful under the original shell, even if there was as much
virtual memory available as required.

Now, I'm happy to confess that I don't know where the inconsistency
Geoff discovered originates, he may well be right that it stems
from poor kernel behaviour, but I'm not convinced.  The modified
shell seems to work correctly in both instances of:

term% srv myrtle myrtle /n/kremvax
post...
term% cd /n/kremvax/usr/share/sendmail
term% echo */.
LICENSE/. README/. cf/. domain/. feature/. hack/. m4/. mailer/. ostype/. sh/. siteconfig/.
term% /sys/src/cmd/rc/8.out -c 'echo */.'
cf/. domain/. feature/. hack/. m4/. mailer/. ostype/. sh/. siteconfig/.
term% /sys/src/cmd/rc/8.out -c 'echo */..'
cf/.. domain/.. feature/.. hack/.. m4/.. mailer/.. ostype/.. sh/.. siteconfig/..
term% echo */..
cf/.. domain/.. feature/.. hack/.. m4/.. mailer/.. ostype/.. sh/.. siteconfig/..
term%

The above mirrors the Unix (NetBSD) shell behaviour (I use pdksh,
incidentally) which, interestingly, the Plan 9 version does not:

term% ape/psh
bind: /386/bin/pub: '/386/bin/pub' does not exist
$ pwd
/n/kremvax/usr/share/sendmail
$ echo */
cf/ domain/ feature/ hack/ m4/ mailer/ ostype/ sh/ siteconfig/
$ echo */.
LICENSE/. README/. cf/. domain/. feature/. hack/. m4/. mailer/. ostype/. sh/. siteconfig/.
$ echo */..
cf/.. domain/.. feature/.. hack/.. m4/.. mailer/.. ostype/.. sh/.. siteconfig/..
$

So, I see that I may be chasing the wrong wild goose, but I'm only
tangentially masking what may be a kernel error.  The changes to
rc just happened to be misguided, initially, and accidentally caused
the additional problem to be fixed once I corrected my poor
understanding.  I would certainly be curious to get to the bottom
of Geoff's anomaly.

++L

Re: [9fans] Ugly hack

[David Butler]

Please forgive this outburst, but I just have to say something...

The crux of the problem is:

[snip]

> makes rc's behaviour consistent
> with my Unix-based expectations

[snip]

Repeat after me:

"Plan 9 is not Unix and in fundamental ways can NOT,
and SHOULD NOT, be made to look like Unix."

What happens when you do echo /*/*/*/*/*/*/*/* in Windows? You
get /*/*/*/*/*/*/*/*. I guess it is not Unix. But if you run the GNU
tools....


I think the biggest problem with Plan 9 is it looks too much like Unix
and Bell Labs distributed APE. I'm glad they are generous with their
code, but I think APE is Plan 9's Pandora's box. I watch with horror
to see X, Perl and GNU tools "ported" to Plan 9. Linux was invented
to give the Unix world a toy. Play there.

I have argued this point for a long time and have recommended changes
that make Plan 9 even less POSIX like because it would make Plan 9
a better computing platform. Get out of the box.

Free Your Mind, Neo! There is no spoon.

Re: [9fans] Ugly hack

[Russ Cox]

> Well, the shell does not make use of any special kernel features
> to expand globbed arguments, it recursively descends the directory
> tree.  I'm addressing that aspect and I'm quite pleased with the
> results.

My impression was that you were trying to make

	echo */
and
	echo */.

match only the directories.  The fact that

	access("/adm/users/.", 0)

succeeds is a kernel bug and not something the shell
should be trying to hack around.  The dotdot paper
shows a good example (ksh) of what confusion
results when user-level programs try to fix kernel bugs
(in that case, the handling of dotdot).

Put another way, the definition of */. is (on Unix
or on Plan 9) all the valid paths you get by substituting
a name in the current directory for the star.
It so happens that this set is different on Plan 9,
and that is in fact a bug caused by early cleaning
of the names, and nothing more.  It's not the shell's
fault, and it's certainly not the shell's place to patch
it up.

In the process you appear to have improved rc's
performance somewhat, by using the extra information
you get from a Plan 9 readdir as opposed to a Unix readdir,
and that's nice, but not what I was referring to.

Russ

Re: [9fans] Ugly hack

[Lucio De Re]

On Sun, Feb 16, 2003 at 10:40:31AM -0500, Russ Cox wrote:
>
> My impression was that you were trying to make
>
> 	echo */
> and
> 	echo */.
>
> match only the directories.  The fact that
>
Agreed.  The original intent was indeed that.

> 	access("/adm/users/.", 0)
>
> succeeds is a kernel bug and not something the shell
> should be trying to hack around.  The dotdot paper
> shows a good example (ksh) of what confusion
> results when user-level programs try to fix kernel bugs
> (in that case, the handling of dotdot).
>
Appreciated.  That was the problem raised by Geoff and he was quite
right to surmise where the problem originated.  However, the change I
made to the shell was flawed, so I both fixed my error _and_
accidentally bypassed the kernel flaw.  I wasn't aiming to do the
latter.

> Put another way, the definition of */. is (on Unix
> or on Plan 9) all the valid paths you get by substituting
> a name in the current directory for the star.
> It so happens that this set is different on Plan 9,
> and that is in fact a bug caused by early cleaning
> of the names, and nothing more.  It's not the shell's
> fault, and it's certainly not the shell's place to patch
> it up.
>
Accepted.  But I've no idea how to fix it, so I leave that to superior
intellects.  That should cause pdksh to come right of its own accord.

> In the process you appear to have improved rc's
> performance somewhat, by using the extra information
> you get from a Plan 9 readdir as opposed to a Unix readdir,
> and that's nice, but not what I was referring to.
>
Indeed.  As long as we agree that my changes to the shell were not
aimed at fixing Geoff's anomaly, but rather to fix my poor
understanding of the shell code in the initial attempt (which failed
miserably on /bin/*/*, incidentally).

++L

Re: [9fans] Ugly hack

[John Kodis]

In article <20030215174605.I9084@cackle.proxima.alt.za>, Lucio De Re wrote:

>  I've found the ability to limit shell command line argument expansion
>  (only in English can one write something so succintly and so ugly) to
>  directories rather than all directory entries by suffixing the last
>  part with a "/".  This is normal behaviour in the NetBSD shells.

You could say this in an even uglier and more succinct fashion by
writing "globbing" instead of "shell command line argument expansion".

>  broken! echo rdesk*
>  rdesktop-1.1.0 rdesktop-1.1.0.tar
>  broken! cd rdesk*/
>  broken! pwd
>  /usr/lucio/Projects/Sundry/rdesktop-1.1.0

While I recognize that this would be a generally useful mechanism, in
this particular case, I'd just use filename completion: I'd type "cd
rdesk<TAB>", this gets completed to rdesktop-1.1.0, I hit return and
I'm done.

If you're interested in prior art in this area, you may want to look
over the man page for zsh glob qualifiers at
http://zsh.sourceforge.net/Doc/Release/zsh_13.html#SEC62

In zsh, *(/) expands to all directories, *(*) expands to a executable
plain files, *(W,X) expands to all world-writable or world-executable
files, and the complexity spirals rapidly out of control from there.

--
John Kodis                                    Goddard Space Flight Center
kodis@mail630.gsfc.nasa.gov                      Greenbelt, Maryland, USA