Re: ls and root directory indicator (original) (raw)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

From: Eric Blake
Subject: Re: ls and root directory indicator
Date: Mon, 25 Feb 2013 15:58:02 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/20130219 Thunderbird/17.0.3

On 02/25/2013 12:51 PM, Sami Kerola wrote:

Did I understood correctly that a patch such below would be more preferred?

Closer, but not quite there yet.

Secondly,pardon my ignorance, I thought '/' and '//' or how ever many slashes are the same root.

"///", "////", and any other larger number of other slashes are all the same as "/". Only "//" is special.

Is this some non-obvious portability gotcha? A link to education material would be great.

POSIX says: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap04.html#tag_04_12

"A pathname consisting of a single shall resolve to the root directory of the process. A null pathname shall not be successfully resolved. A pathname that begins with two successive characters may be interpreted in an implementation-defined manner, although more than two leading characters shall be treated as a single character."

Linux defines "//" to be a synonym for "/", which is why lots of people fail to realize the portability issue. But Cygwin defines "//" to be a separate root, where "//machine/share" is the preferred way to access remote share drives. There, 'ls //' shows a list of accessible remote machines (speaking from personal experience, that is nice for small home networks, but painfully slow on large corporate networks)

@@ -3970,7 +3971,7 @@ printlongformat (const struct fileinfo *f) printtypeindicator (true, f->linkmode, unknown); } } - else if (indicatorstyle != none) + else if (indicatorstyle != none && isrootonly(f->name)) printtypeindicator (f->statok, f->stat.stmode, f->filetype);

This logic feels backwards - you want to print_type_indicator() on non-root, but when reading the code as written, it looks like you are only printing indicators on roots.

}

@@ -4212,7 +4213,7 @@ printfilenameandfrills (const struct fileinfo *f, sizet startcol)

sizet width = printnamewithquoting (f, false, NULL, startcol);

- if (indicatorstyle != none) + if (indicatorstyle != none && isrootonly(f->name)) width += printtypeindicator (f->statok, f->stat.stmode, f->filetype);

Again, this logic feels backwards.

return width; @@ -4397,6 +4398,21 @@ putindicator (const struct binstr *ind) fwrite (ind->string, ind->len, 1, stdout); }

+/* Find out if the output name is root, and represent it as a single + * slash even if file type indication is requested. */ +static int GLATTRIBUTEPURE +isrootonly (const char *path) +{ + while (*path) + { + if (*path == '/') + path++; + else + return 1;

Ah, that explains why your logic feels backwards. Based on your choice of names, I was expecting true (non-zero) for root, false (zero) if only slashes were seen, but you did the exact opposite.

We prefer bool over int for predicate functions that only return a yes or no answer. Also, detecting roots is shorter (less code, and potentially faster operation if libc optimized strspn() to do word-at-a-time searching instead of byte-at-a-time) when done as:

static bool _GL_ATTRIBUTE_PURE is_root (const char *path) { return path[strspn(path, "/")] == '\0'; }

Hmm, that form is optimized to POSIX; it might need a bit more hand-holding to be portable to mingw, where drive letters are also treated as roots, and where backslash can be used in place of slash. But I'm okay if we leave the case of mingw to someone that develops on that platform; again, isolating things into an is_root() helper function already gives them a nice place to tackle if they want to enhance the function to work on drive letters.

-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature