Bug 33187
| Summary: | [PATCH] /bin/ls -dF adds trailing slash regardless of whether there is already one there | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | Base System | Reporter: | mikem <mike_makonnen> | ||||||
| Component: | bin | Assignee: | ru <ru> | ||||||
| Status: | Closed FIXED | ||||||||
| Severity: | Affects Only Me | ||||||||
| Priority: | Normal | ||||||||
| Version: | 5.0-CURRENT | ||||||||
| Hardware: | Any | ||||||||
| OS: | Any | ||||||||
| Attachments: |
|
||||||||
|
Description
mikem
2001-12-26 09:50:01 UTC
That patch really didn't sit well with me, so I wrote another one. It was so simple that I wonder why I didn't think of it in the first place. Hmm... maybe it's all that X-mas liquor :-) Anyways, the patch is attached to this email Cheers, mikem ok, let's try this one more time...
--- /usr/src/bin/ls/ls.c Tue Nov 27 06:30:05 2001
+++ ls.c Wed Dec 26 03:35:50 2001
@@ -132,7 +132,7 @@
{
static char dot[] = ".", *dotav[] = { dot, NULL };
struct winsize win;
- int ch, fts_options, notused;
+ int ch, fts_options, notused, index, len;
char *p;
#ifdef COLORLS
@@ -382,6 +382,19 @@
printfcn = printlong;
else
printfcn = printcol;
+
+ /*
+ * Remove trailing '/' if -d and -F are specified
+ * so we don't get into the situation where:
+ * % ls -dF /usr/
+ * outputs: /usr//
+ */
+ if (f_listdir && f_type)
+ for(index=0; index < argc ; index++) {
+ len = strlen(argv[index]);
+ if (argv[index][len - 1] == '/')
+ argv[index][len - 1] = '\0';
+ }
if (argc)
traverse(argc, argv, fts_options);
Responsible Changed From-To: freebsd-bugs->sheldonh I'll take this one. On Wed, 26 Dec 2001 03:50:02 PST, mikem wrote: > ok, let's try this one more time... I like this. :-) The following variant of your patch makes small stylistic changes: > - int ch, fts_options, notused; > + int ch, fts_options, notused, index, len; These new variables should follow the sort order, and index is usually called i. > + /* > + * Remove trailing '/' if -d and -F are specified > + * so we don't get into the situation where: > + * % ls -dF /usr/ > + * outputs: /usr// > + */ This comment doesn't look like the rest of the comments. Ciao, Sheldon. Index: ls.c =================================================================== RCS file: /home/ncvs/src/bin/ls/ls.c,v retrieving revision 1.51 diff -u -d -r1.51 ls.c --- ls.c 29 Dec 2001 00:22:29 -0000 1.51 +++ ls.c 9 Jan 2002 13:34:48 -0000 @@ -135,7 +135,7 @@ { static char dot[] = ".", *dotav[] = {dot, NULL}; struct winsize win; - int ch, fts_options, notused; + int ch, fts_options, i, len, notused; char *p; #ifdef COLORLS char termcapbuf[1024]; /* termcap definition buffer */ @@ -392,6 +392,17 @@ printfcn = printlong; else printfcn = printcol; + + /* + * If -d and -F options, strip trailing slashes from arguments + * to avoid duplicated terminating slashes in output. + */ + if (f_listdir && f_type) + for(i=0; i < argc ; i++) { + len = strlen(argv[i]); + if (argv[i][len - 1] == '/') + argv[i][len - 1] = '\0'; + } if (argc) traverse(argc, argv, fts_options); On Wed, Jan 09, 2002 at 03:51:13PM +0200, Sheldon Hearn wrote: > > Hi folks, > > What do you think of the following patch, taken from PR bin/33187? > > It prevents > > ls -dF /usr/ > > from producing > > /usr// > > which may confuse other programs for which this output is used as input. > It also doesn't work for ``ls -dF /usr//'' and breaks ``ls -dF /''. > Index: ls.c > =================================================================== > RCS file: /home/ncvs/src/bin/ls/ls.c,v > retrieving revision 1.51 > diff -u -d -r1.51 ls.c > --- ls.c 29 Dec 2001 00:22:29 -0000 1.51 > +++ ls.c 9 Jan 2002 13:34:48 -0000 > @@ -135,7 +135,7 @@ > { > static char dot[] = ".", *dotav[] = {dot, NULL}; > struct winsize win; > - int ch, fts_options, notused; > + int ch, fts_options, i, len, notused; > char *p; > #ifdef COLORLS > char termcapbuf[1024]; /* termcap definition buffer */ > @@ -392,6 +392,17 @@ > printfcn = printlong; > else > printfcn = printcol; > + > + /* > + * If -d and -F options, strip trailing slashes from arguments > + * to avoid duplicated terminating slashes in output. > + */ > + if (f_listdir && f_type) > + for(i=0; i < argc ; i++) { > + len = strlen(argv[i]); > + if (argv[i][len - 1] == '/') > + argv[i][len - 1] = '\0'; > + } > > if (argc) > traverse(argc, argv, fts_options); Cheers, -- Ruslan Ermilov Oracle Developer/DBA, ru@sunbay.com Sunbay Software AG, ru@FreeBSD.org FreeBSD committer, +380.652.512.251 Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age State Changed From-To: open->suspended There doesn't seem to be an easy way to handle this. For now, it's probably better to simply fix scripts that use ls -dF output. For the record, I'm pretty sure I remember POSIX warning against relying on the output of ls(1) in scripts. Responsible Changed From-To: sheldonh->freebsd-bugs Leave this one for another brave soul. On Wed, 9 Jan 2002, Ruslan Ermilov wrote:
> On Wed, Jan 09, 2002 at 03:51:13PM +0200, Sheldon Hearn wrote:
> >
> > Hi folks,
> >
> > What do you think of the following patch, taken from PR bin/33187?
> >
> > It prevents
> >
> > ls -dF /usr/
> >
> > from producing
> >
> > /usr//
> >
> > which may confuse other programs for which this output is used as input.
> >
> It also doesn't work for ``ls -dF /usr//'' and breaks ``ls -dF /''.
It also breaks ls of symlinks. E.g.:
$ ls -lF /var/crash /var/crash/
lrwxr-xr-x 1 root wheel 8 Mar 5 2001 /var/crash@ -> /c/crash
/var/crash/:
total 145202
-rw-r--r-- 1 4294967294 wheel 2 Jan 9 14:44 bounds
-rw-r--r-- 1 4294967294 wheel 2630180 Jan 9 14:45 kernel.4
-rw-r--r-- 1 root wheel 5 May 19 1994 minfree
-rw------- 1 4294967294 wheel 14680064 Nov 24 21:06 vmcore.1
-rw------- 1 4294967294 wheel 1048576 Nov 24 21:09 vmcore.2
-rw------- 1 4294967294 wheel 7053312 Jan 7 14:16 vmcore.3
-rw------- 1 4294967294 wheel 268435456 Jan 9 14:45 vmcore.4
I think the slash should be stripped in the output at most.
Bruce
On Wed, 9 Jan 2002 16:19:06 +0200 Ruslan Ermilov <ru@freebsd.org> wrote: > > > It also doesn't work for ``ls -dF /usr//'' and breaks ``ls -dF /''. > Ok, the issue is a bit more complicated than I thought at first. Anyways, regardless of the patch, it seems ls(1) accepts the following: ``ls -dF /usr////////////////''. It seems like ls(1) (or rather the fts_* family of functions) doesn't care how many trailing '/' there are. Is this a possible bug in fts_*? Anyone know what POSIX has to say about this? Cheers, mike makonnen On Thu, 10 Jan 2002 15:04:31 +1100 (EST) Bruce Evans <bde@zeta.org.au> wrote: > It also breaks ls of symlinks. E.g.: > > $ ls -lF /var/crash /var/crash/ > lrwxr-xr-x 1 root wheel 8 Mar 5 2001 /var/crash@ -> /c/crash > > /var/crash/: > total 145202 > -rw-r--r-- 1 4294967294 wheel 2 Jan 9 14:44 bounds > -rw-r--r-- 1 4294967294 wheel 2630180 Jan 9 14:45 kernel.4 > -rw-r--r-- 1 root wheel 5 May 19 1994 minfree > -rw------- 1 4294967294 wheel 14680064 Nov 24 21:06 vmcore.1 > -rw------- 1 4294967294 wheel 1048576 Nov 24 21:09 vmcore.2 > -rw------- 1 4294967294 wheel 7053312 Jan 7 14:16 vmcore.3 > -rw------- 1 4294967294 wheel 268435456 Jan 9 14:45 vmcore.4 I realize the pr has been suspended, but just to set the record straight... this is not caused by the patch. The patch only comes into effect when both -d and -F are specified. > I think the slash should be stripped in the output at most. Yeah, you're right. I also just found out ls(1) will accept ``ls /usr////////''. So, should all trailing '/' be stripped on output or is it not worth the effort/POSIX compliance/whatever ? Cheers, mike makonnen On Thu, 10 Jan 2002, Mike Makonnen wrote: > On Thu, 10 Jan 2002 15:04:31 +1100 (EST) > Bruce Evans <bde@zeta.org.au> wrote: > > > It also breaks ls of symlinks. E.g.: > > > > $ ls -lF /var/crash /var/crash/ > > lrwxr-xr-x 1 root wheel 8 Mar 5 2001 /var/crash@ -> /c/crash > > > > /var/crash/: > > total 145202 > > -rw-r--r-- 1 4294967294 wheel 2 Jan 9 14:44 bounds > > -rw-r--r-- 1 4294967294 wheel 2630180 Jan 9 14:45 kernel.4 > > -rw-r--r-- 1 root wheel 5 May 19 1994 minfree > > -rw------- 1 4294967294 wheel 14680064 Nov 24 21:06 vmcore.1 > > -rw------- 1 4294967294 wheel 1048576 Nov 24 21:09 vmcore.2 > > -rw------- 1 4294967294 wheel 7053312 Jan 7 14:16 vmcore.3 > > -rw------- 1 4294967294 wheel 268435456 Jan 9 14:45 vmcore.4 > > I realize the pr has been suspended, but just to set the record straight... > this is not caused by the patch. The patch only comes into effect when both -d and -F are specified. I forgot it was -d and my fingers knew it was -l :-). Anyway, "ls -dF /var/crash/" should follow the symlink (if any). Appending a slash is a normal way to get symlinks followed. For ls, you can use -L, but most utilities don't have an equivalent. > > I think the slash should be stripped in the output at most. > > Yeah, you're right. I also just found out ls(1) will accept ``ls /usr////////''. So, should all trailing '/' be stripped on output or is it not > worth the effort/POSIX compliance/whatever ? I doubt that POSIX specifies the output of ls in enough detail to say, but I think stripping should not change the semantics of the pathnames, in case they are used for input. Stripping "//" to "/" changes the semantics, as does stripping "foo/" to "foo" if "foo" is not a directory. Otherwise, trailing slashes may be stripped without changing the semantics. Bruce On Thu, Jan 10, 2002 at 11:38:47PM +1100, Bruce Evans wrote: > On Thu, 10 Jan 2002, Mike Makonnen wrote: > > > On Thu, 10 Jan 2002 15:04:31 +1100 (EST) > > Bruce Evans <bde@zeta.org.au> wrote: > > > > > It also breaks ls of symlinks. E.g.: > > > > > > $ ls -lF /var/crash /var/crash/ > > > lrwxr-xr-x 1 root wheel 8 Mar 5 2001 /var/crash@ -> /c/crash > > > > > > /var/crash/: > > > total 145202 > > > -rw-r--r-- 1 4294967294 wheel 2 Jan 9 14:44 bounds > > > -rw-r--r-- 1 4294967294 wheel 2630180 Jan 9 14:45 kernel.4 > > > -rw-r--r-- 1 root wheel 5 May 19 1994 minfree > > > -rw------- 1 4294967294 wheel 14680064 Nov 24 21:06 vmcore.1 > > > -rw------- 1 4294967294 wheel 1048576 Nov 24 21:09 vmcore.2 > > > -rw------- 1 4294967294 wheel 7053312 Jan 7 14:16 vmcore.3 > > > -rw------- 1 4294967294 wheel 268435456 Jan 9 14:45 vmcore.4 > > > > I realize the pr has been suspended, but just to set the record straight... > > this is not caused by the patch. The patch only comes into effect when both -d and -F are specified. > > I forgot it was -d and my fingers knew it was -l :-). Anyway, "ls -dF > /var/crash/" should follow the symlink (if any). Appending a slash > is a normal way to get symlinks followed. For ls, you can use -L, but > most utilities don't have an equivalent. > > > > I think the slash should be stripped in the output at most. > > > > Yeah, you're right. I also just found out ls(1) will accept ``ls /usr////////''. So, should all trailing '/' be stripped on output or is it not > > worth the effort/POSIX compliance/whatever ? > > I doubt that POSIX specifies the output of ls in enough detail to say, but > I think stripping should not change the semantics of the pathnames, in > case they are used for input. Stripping "//" to "/" changes the semantics, > as does stripping "foo/" to "foo" if "foo" is not a directory. Otherwise, > trailing slashes may be stripped without changing the semantics. > Let's not overload our utilities with the stuff like this. There's nothing wrong with trailing slashes if the user so wanted -- for those who doesn't, a simple sed(1) sequence would be in place, or realpath(3) from within C. More to the point, the multiple slashes in the middle of a pathname are not covered by this PR, and I don't see a reason why we should treat trailing slashes differently. I insist on us closing this PR. Cheers, -- Ruslan Ermilov Oracle Developer/DBA, ru@sunbay.com Sunbay Software AG, ru@FreeBSD.org FreeBSD committer, +380.652.512.251 Simferopol, Ukraine http://www.FreeBSD.org The Power To Serve http://www.oracle.com Enabling The Information Age On Fri, 11 Jan 2002 09:34:14 +0200 Ruslan Ermilov <ru@freebsd.org> wrote: > slashes differently. I insist on us closing this PR. Fine with me. Cheers, mike makonnen On Fri, 11 Jan 2002 09:34:14 +0200, Ruslan Ermilov wrote:
> I insist on us closing this PR.
No objection from me.
Ciao,
Sheldon.
State Changed From-To: suspended->closed Originator agreed this PR can be closed for reasons specified in my last reply. Responsible Changed From-To: freebsd-bugs->ru |
