[Toybox] [PATCH] Implement lsof.

enh enh at google.com
Fri Sep 4 22:24:45 PDT 2015


On Fri, Sep 4, 2015 at 9:13 PM, Rob Landley <rob at landley.net> wrote:
>
>
> On 09/04/2015 12:06 PM, enh wrote:
>> On Fri, Sep 4, 2015 at 8:47 AM, Rob Landley <rob at landley.net> wrote:
>>> On 08/29/2015 03:54 PM, enh wrote:
>>>> Implement lsof.
>>>>
>>>> This is a superset of the current AOSP lsof (which is itself a
>>>> superset of the lsof in Android M). It fixes several bugs/misfeatures
>>>> and adds support for decoding IPv4/IPv6 tcp/udp/raw sockets and Unix
>>>> domain sockets.
>>>>
>>>> I'll remove toolbox lsof as soon as this is available in toybox.
>>>
>>> Added, but could you hold off until monday on swapping over? I'd like to
>>> take a stab at cleaning it up and promoting it this weekend.
>>
>> sure. (though bear in mind that what i sent you is quite different to
>> what we've been using in Android --- all the socket stuff is new, for
>> example. so it still needs to survive its first contact with the
>> enemy.)
>
> Grr, what's a terse way to indicate "comma separated list" in the usage:
> line?
>
> I'm sort of leaning towards:
>
>   usage: lsof [-lt] [-u UID,] -p [PID,] [FILE...]
>
> Where the trailing comma indicates comma separated list since the ...
> means space separated list.
>
> I also may want to teach lib/args.c to parse a , flag because mount -o
> and ps -o can allready occur multiple times, so you can go "ps -o uid -o
> gid" and it means "ps -o uid,gid", and lsof -u is a third command using
> this. What I've _been_ doing was the * logic to collect a struct
> arg_list * and then collating them with a function, but it sounds like
> args.c should collect a string and do its own "," collation?
>
> Yeah, ALLOCATION CONTEXT, but I'm not really that invested in making
> NOFORK work outside of shell builtins, so cleaning up small memory leaks
> on re-exec without actual exec isn't a huge deal? (And really, I could
> parse the same option string to find out if I need to and have a cleanup
> function in args.c if I cared _that_ much.)
>
> No, it's the help text that's the hard part. [-o OPTION,] in mount is
> too _subtle_. And the comma is too easy to miss, but "[-o
> OPTION[,OPTION...]]" is to _verbose_. Right now the mount help text
> explains "comma separated list" outside of the argument list, maybe I
> need to do that all the time?
>
> Hmm, [-o OPTION,,,] and [-o OPTION,...] aren't right either. I want a
> shorthand that doesn't require people to already be familiar with it to
> understand it. Probably the [FILE...] thing is opaque to newbies but I
> lifted _that_ from man pages. Let's see, for mount it says:
>
>        mount [-fnrsvw] [-o option[,option]...]  device|dir
>
> Meanwhile the ps man page has no usage: and doesn't describe -o until
> line 295, so that's nice.
>
> The tar man page... doesn't really describe anything? I'm not entirely
> sure what --exclude PATTERN and --exclude-tag FILE do or how they
> differ. Doe t he trailing comma on the description of --exclude-caches
> mean something, or is it a typo? I'd have to experiment with the
> command. And what does...
>
>      --exclude-tag=FILE
>            exclude contents of directories containing FILE, except
>
> Mean? I mean, was there a rendering error in the man page, or is it
> referring to the command after it, or...? (Xubuntu 14.04, as usual.)
>
> Right, going with trailing comma for the moment

i came up with yet another option, but i think any are fine. (and we
both seem to agree that we should explicitly state that it's a
comma-separated list in the option-specific help.)

>>> For one thing I have a lsof.txt here with:
>>>
>>>   names argument: restrict open files named to list of filenames
>>>   -l turn off username/group lookup for display
>>>   -u USR restrict processes examined to specified list of users
>>>   -p PIDS only processes in PID list (or ^PID to exclude that pid)
>>>   -t (thing android host scripts use?)
>>>
>>> So I should probably tackle adding that before people depend on the code
>>> being stable. :)
>>
>> i can implement those too if you want to go back to ps :-)
>
> Eh, I've got the bit in my teeth now. The spacing is essentially
> repeated between print_header() and print_info() but print_header() is
> almost print_info() with a different array.

well, the key thing there is that the spacing is just [roughly] what
the Android lsof had. the real lsof sizes [some of] the columns based
on the content. i left a TODO to that effect.

> And while multiple -p can
> use lsof_pid(), scanning for -u or FILE arguments basically needs to do
> a full traverse and have filters.
>
> So does -i. I wanna do -i to because I use that one a lot. (What's bound
> to port 6903 _now_?) Looks fairly straightforward since you're producing
> the output and we just want to filter, except...

i didn't actually know about those options until i was reading the
help tonight. i've always just used grep. (and i suspect it would take
me longer to remember this syntax each time than to just use the same
big hammer every time. i actually use grep more than filename
filtering because then i don't have to worry about the full path.)

> Option parsing again. -i4 and -i6. (With -i being a chord of -i4 and -i6)
>
> Well, ok... I can _cheat_. -i as a flag, -4 as a flag, -6 as a flag, and
> [-i46] as a group. (The behavior would be right. It would accept certain
> crazy inputs, but not _wrong_, just... easter-eggish.)

lsof's entire command-line syntax is easter-eggish.

> Stuff!
>
> Rob



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.

 1441430685.0


More information about the Toybox mailing list