This is the mail archive of the
gdb-patches@sources.redhat.com
mailing list for the GDB project.
Re: [rfa] linespec.c, part 5
On Fri, 15 Nov 2002 20:08:27 -0500, Elena Zannoni <ezannoni@redhat.com> said:
> David Carlton writes:
>> * The function that this creates, decode_compound, is quite long.
>> Later patches in this series will break it down into smaller
>> chunks; they will come after I've got decode_line_1 itself down to
>> a nice size.
> OK but, could you add something to the comment at the top of the
> function that describes what the function returns and what the
> params are?
Sure, I can elaborate on that comment. Though I don't think I want to
describe _all_ of the args there: many functions get passed the args
of decode_line_1, so I'd rather put a single comment describing that
somewhere rather than repeating that over and over again.
Maybe I'll replace the these comments right after the end of
decode_line_1:
/* Now, still more helper functions. */
/* NOTE: carlton/2002-11-07: Some of these have non-obvious side
effects. In particular, if a function is passed ARGPTR as an
argument, it modifies what ARGPTR points to. (Typically, it
advances *ARGPTR past whatever substring it has just looked
at.) */
with a comment saying:
/* Now, more helper functions for decode_line_1. Some conventions
that these functions follow:
Decode_line_1 typically passes along some of its arguments or local
variables to the subfunctions. It passes the variables by
reference if they are modified by the subfunction, and by value
otherwise.
Some of the functions have side effects that don't arise from
variables that are passed by reference. In particular, if a
function is passed ARGPTR as an argument, it modifies what ARGPTR
points to; typically, it advances *ARGPTR past whatever substring
it has just looked at. (If it doesn't modify *ARGPTR, then the
function gets passed *ARGPTR instead, which is then called ARG: see
set_flags, for example.) Also, functions that return a struct
symtabs_and_lines may modify CANONICAL, as in the description of
decode_line_1.
If a function returns a struct symtabs_and_lines, then that struct
will immediately make its way up the call chain to be returned by
decode_line_1. In particular, all of the functions decode_XXX
calculate the appropriate struct symtabs_and_lines, under the
assumption that their argument is of the form XXX. */
Is that clearer?
>> * Since decode_line_1 returns the value of decode_compound, there's no
>> need to check whether or not decode_compound modifies the arguments
>> that it's been passed: later code in decode_line_1 can never be
>> effected by such modifications.
> Hmmm. maybe I am not understanding what you say. Does
> decode_compound modify its args? seems like it changes argptr and
> canonical. What if somebody changes decode_line_1 in the future so
> that there is more code executed after the call, it could probably
> cause some head scratching to see that the call didn't leave things
> as expected. I haven't looked too closely though.
Does the above comment help? What I meant was: every code path in the
code that I extracted leads to either a return from decode_line_1 or
an error. So, for example, subsequent code in decode_line_1 won't
depend on, say, modifications to 'p' within decode_compound.
Decode_compound prepares ARGPTR and CANONICAL appropriately for
return, and calculates the struct symtabs_and_lines to be returned.
If somebody subsequently decides that, say, a bit more fiddling should
happen within decode_line_1, I think these changes should make that
fiddling a lot easier rather than a lot harder.
It will also help once I've renamed some variables, so 's' turns into
'file_symtab', 'p' turns into 'next_component', and so forth. That
will make it much easier for future readers of the code to understand
the possible ramifications of changing the values of those variables.
(Whereas who knows what it means to change the value of a variable 'p'
that is referred to in a zillion places, with some of those uses
distinct and some of them not.)
David Carlton
carlton@math.stanford.edu