Re: [PATCH] ls - colorize files with capabilities

[Jim Meyering]

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.