[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] ls - colorize files with capabilities
From: |
Jim Meyering |
Subject: |
Re: [PATCH] ls - colorize files with capabilities |
Date: |
Mon, 21 Jul 2008 17:43:52 +0200 |
Kamil Dudka <address@hidden> wrote:
...
>> Move this cap_free call "up". Then you can remove
>> the repeated call below.
>>
>> > + return false;
>> > + }
>> > +
>> > + /* check if human-readable capability string is empty */
>> > + hasCap = *result;
> I am not sure about this change. libcap API is not well documented, so I
> decided to take code directly from getcap.c(do_getcap) - this utility is
> already tested by users. In other case I am afraid of possible premature free
> of libcap resource.
Don't pessimize your code because doing it cleanly might not work.
Have no fear ;-), and do it cleanly.
The documentation ("man cap_to_text") appears to guarantee
the required semantics, so it ought to work.
If in doubt, test it with valgrind.
...
> /* Returns whether any color sequence was printed. */
> static bool
> print_color_indicator (const char *name, mode_t mode, int linkok,
> @@ -3923,6 +3963,8 @@ print_color_indicator (const char *name, mode_t mode,
> int linkok,
> type = C_SETUID;
> else if ((mode & S_ISGID) != 0)
> type = C_SETGID;
> + else if (has_capability (name))
> + type = C_CAP;
> else if ((mode & S_IXUGO) != 0)
> type = C_EXEC;
> }
Oops. Mangled indentation.
BTW, I noticed something similar in your dd patch.
Do you use unusual TAB-stop settings?
Otherwise, so far, so good. Whenever we add a new ls-color-related
capability, the corresponding list in dircolors.c must be updated
to match. Otherwise, "make check-ls-dircolors" will fail. Also,
dircolors.hin must be updated to add a brief description.
And of course, we'll need at least one test.
To test it, you'll have to require a cap-related
command-line tool (i.e., skip the test if it doesn't
exist -- there are similar functions in test-lib.sh,
e.g., require_strace_ and require_acl_), and then run
LS_COLORS='...' ls --color=always file-with-cap > out
and confirm that the expected (new) mark-up is present.
See tests/ls/stat-free-symlinks for a similar test that
runs ls with a custom LS_COLORS setting.
And here you thought this would be simple.
Thanks for persevering.