On Mon, Mar 09, 2009 at 01:26:41PM -0700, Dan Price wrote:
> On Mon 09 Mar 2009 at 12:21PM, Jonathan Adams wrote:
> > The main effect of the currently proposed kmem walker is to unify the
> > 6 kmem walkers: "kmem", "bufctl", "free", "freectl", "free_constructed",
> > "freectl_constructed" into a single "kmem" walker, and adding the ability
> > to walk all of the different varieties from the per-cache walkers. So
> > you can do:
> >
> > ::walk kmem_alloc_8192 - -f
> >
> > to walk the free buffers in kmem_alloc_8192. (the '-' is for the
> > not-well-known "variable" argument to ::walk)
>
> While this makes plenty of sense, I think this particular bit
> of syntax is going to trip up a lot of people who are not experts
> at this sort of thing. I have a couple of questions about it, as
> a result:
>
> 1) Should we change the syntax? I can think of at least
> two approaches. I'm not in love with either....
>
> ::walk [-something <varname>] <walker> [walker options]
>
> Or splitting ::walk into '::walk [options]' and
> '::walkvar <var> [options]'
I've gone back and forth on this, and those were the other options I saw.
Maybe doing an incompatible change is the right answer:
::walk [-v var] <walker> [args]
This is something which is rarely used, and a getopt-style option is a better
way to handle it. I shied away from it because of the incompatible change
to a committed interface.
> 2) I saw that your code supports:
>
> ::walk kmem_alloc_8192 - -f
>
> and
>
> ::walk kmem_alloc_8192 -- -f
>
> It seems like the latter form is better? It makes more
> sense to me from a getopt compatibility perspective.
It's a little weird either way; I'm fine with pushing for -- in all of the
documentation.
> 3) If not (1), then could we detect likely mistakes and tell
> the user what to do?
>
> Today:
>
> > ::walk kmem_alloc_8192 -f
> mdb: '-' may not be used in a variable name
>
> Now, looking at your code, I expect it will print the same
> thing-- but as a result, the message is effectively telling
> the user that passing a '-' is going to be illegal. Could we
> do something like this?
>
> > ::walk kmem_alloc_8192 -f
> mdb: '-' may not be used in a variable name.
> mdb: did you mean: ::walk kmem_alloc_8192 -- -f?
That's easy enough to do.
Cheers,
- jonathan
