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]

[RFC/RFA] add struct parse_context to all command functions


Hello,

I'd like to start working on adding a struct parse_context again.
The ultimate goal is to pass this structure as an argument to all
the "parse..." routines, rather than rely on the current_language
and input_radix globals.  In addition to being cleaner, it will also
help fix a bug where the current_language is switched under us while
trying to do parse an expression.

The first thing was to define the parse_context struct.  Since a very
large number of .c files add new commands, I decided to put the struct
in defs.h.  The alternative was to put it in expression.h, besides the
parse_* routines, or perhaps in command.h, or in language.h.  Putting
it in defs.h minimizes the size of the patch by avoiding an extra include.

The next step for this task is to add a struct parse_context parameter
to the "command" functions (the functions being called when the user
calls a function in the UI).  When the user enters a command, we know
that we can use the current language and input radix to parse the
arguments. So we create a struct parse context there, and pass it
to the command function.  The goal is to eventually have only one place
that uses the parse context globals: the UI interpreter that calls
the function commands.  Note that I decided to pass a struct rather
than a pointer to the struct to avoid any memory management issue.

I plan to propagate the parse_context argument progressively. But
the first step, which is to add the parse_context argument to the
command functions, is massive. It affects almost every .c file in
the project, so I'd like to make sure that everyone is happy with
what the change looks like.  For this, I will attach a patch that
changes the "add_cmd" et al interface to add the parse_context
argument to the command functions. The patch also contains the
associated adjustements in the implementation of these functions,
as well as one random .c file updated to include the new parameter.

My goal is to get an agreement on the principle, then spent the time
to do the entire transition, and then commit either immediately or
shortly after that. I don't mind posting the patch and let it sit
for a few days, but it's going to be massive, and I don't think that
this is going to bring much value.  But I'm ok with doing that if
people prefer.

If you look at the patch, you'll probably notice a few things that
I would like to fix before doing the transition for real:

  1. The prototypes for the various add_* functions are duplicated.
     There is a copy in command.h, and another in cli/cli-decode.h.
     Is there a specific reason why we can't keep only one copy?
     For instance, only keep the copy in cli/cli-decode.h?

  2. Most if not all add_ command define the profile of the callback
     on the fly, like this:
 
     | add_prefix_cmd (char *, enum command_class,
 !!->|                 void (*fun) (char *, int, struct parse_context),
     |                 char *, struct cmd_list_element **, char *, int,
     |                 struct cmd_list_element **);

     There is the following typedef:

     | typedef void cmd_cfunc_ftype (char *args, ...)

     We should really use this typedef instead of repeating it everywhere.
  
  3. There is one function "add_com" which seems to be a shortcut
     for add_cmd:

       add_com (char *name, enum command_class class,
                void (*fun) (char *, int, struct parse_context),
                char *doc)
       {
         return add_cmd (name, class, fun, doc, &cmdlist);
       }

     The only difference is that one doesn't have to pass the cmdlist
     to add_com.  Do we really want to keep it?  I have always been
     confused trying to find the difference between the two...

  4. I noticed a few command functions that are declared global and
     yet should be static.  I checked insight, and there are only used
     locally.  I will fix them separately.

Voila voila. I will start working on the little issues mentioned above
while waiting for feedback.  I would like to start working on this
no later than, say, a week from now.  And I will provide gdbint
documentation as well as requested by Eli when we discussed this idea
the last time.

-- 
Joel

Attachment: parse_context.diff
Description: Text document


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