[PATCH v2] lsof: correct check for symbolic link

[Thomas De Schampheleire]

On Fri, Jun 21, 2013 at 7:24 PM, Thomas De Schampheleire
<patrickdepinguin+busybox at gmail.com> wrote:
> On Fri, Jun 21, 2013 at 6:05 PM, Mike Frysinger <vapier at gentoo.org> wrote:
>> On Friday 21 June 2013 01:52:20 Thomas De Schampheleire wrote:
>>> --- a/procps/lsof.c
>>> +++ b/procps/lsof.c
>>> @@ -61,12 +61,24 @@ int lsof_main(int argc UNUSED_PARAM, cha
>>>               d_fd = opendir(name);
>>>               if (d_fd) {
>>>                       while ((entry = readdir(d_fd)) != NULL) {
>>> -                             if (entry->d_type == DT_LNK) {
>>> -                                     safe_strncpy(name + baseofs, entry->d_name, 10);
>>> -                                     fdlink = xmalloc_readlink(name);
>>> -                                     printf("%d\t%s\t%s\n", proc->pid, proc->exe, fdlink);
>>> -                                     free(fdlink);
>>> +                             safe_strncpy(name + baseofs, entry->d_name, 10);
>>> +
>>> +                             if (entry->d_type == DT_UNKNOWN) {
>>
>> it's too bad busybox doesn't have unlikely() as this would be a good place to
>> slap that
>>
>> i would rewrite the code a little to avoid the constant strncpy (assuming it
>> doesn't severely impact code size):
>>         while (...) {
>>                 if (entry->d_type != DT_UNKNOWN && entry->d_type != DT_LNK)
>>                         continue;
>>
>>                 safe_strncpy(name + baseofs, entry->d_name, 10);
>>                 if (entry->d_type == DT_UNKNOWN) {
>>                         ... new lstat logic ...
>>                 }
>>
>>                 ... readlink + printf ...
>>         }
>
> Thanks for the feedback, I just sent a third version.
>
> For my info: is the check on the symbolic link only present to skip
> '.' and '..', or are there situations where /proc/<pid>/fd contains
> other entries than symbolic links and the two default directory
> entries?

If it is only to skip . and .., and if we make an additional
assumption that there will not be hidden files in /proc/<pid>/fd, then
the entire code can be reduced to:

                        while ((entry = readdir(d_fd)) != NULL) {
                                if (entry->d_name[0] == '.')
                                        continue;

                                safe_strncpy(name + baseofs, entry->d_name, 10);
                                fdlink = xmalloc_readlink(name);
                                printf("%d\t%s\t%s\n", proc->pid,
proc->exe, fdlink);
                                free(fdlink);
                        }

Which is back at the same size and clarity of the original code.