This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] new command to search memory


On Thu, Feb 28, 2008 at 06:44:00PM -0800, Doug Evans wrote:
> >  The slashed arguments work analagously to x and display, which is
> >  nice.  Should the default count should be one instead of infinity?
> >  I suppose having it default to infinity is nice, since we don't
> >  have to invent a syntax for infinity that way.
> 
> /u or some such for "unlimited" would work I guess.  I can change the
> default if that's preferable.

I think it's fine the way you've got it.

> >  What do you think of "+" instead of "@" to distinguish lengths?  "find
> >  &hello[0] +0x100".
> 
> I picked "@" because it's used, for example, in "p foo[0]@10".  It's
> not identical, but it seemed similar enough.  "+" works too.

I'd prefer +, if that works for you.

> Ya, but for completeness sake it's not just the malloc call, it's the
> whole shebang.  I can understand why in
> 
> (gdb) p strcmp (foo, "bar")
> 
> one wants to download "bar" to the target before calling strcmp, but
> find's needs are different.

Right.  I've got a patch to cover the downloading.  I didn't get it
posted yet but it hasn't dropped off my list for this week either.

> >  qSearch:memory does not need to be advertised for qSupported.  The
> >  rule of thumb is that things which are used to implement a user
> >  command don't need to be, since there's no big penalty if we try them
> >  and are told they are not supported - we'll just try another approach
> >  and next time we'll know.  That means you need to handle
> >  PACKET_DISABLE twice, before and after sending the packet.
> 
> I found a use for the option that goes with qSupported both for
> testing and analysis.  Maybe users would also find use for the choice,
> but it can be tossed.

The option isn't actually linked to qSupported; you can add just the
option, and it will default to auto.

> >  > +If the value size is not specified, it is taken from the
> >  > +value's type.  This is useful when one wants to specify the search
> >  > +pattern as a mixture of types.
> >
> >  IMO this will confuse users for constants, which have type int (or
> >  sometimes long), so could you add a word about that?  Otherwise
> >  someone will type "find &hello[0], @100, 0x65, 0x66" and be confused
> >  by the lack of matches.
> 
> Or one could default to something else, bytes or ints or some such,
> and have a /t option or some such that says to use the type of the
> object.

Either way; I've no preference.  The way you've got it seems fine if
we can clarify the description.

-- 
Daniel Jacobowitz
CodeSourcery


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]