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: [PATCH v3 06/34] Introduce interpreter factories


On 16-05-06 08:34 AM, Pedro Alves wrote:
> +/* See interps.h.  */
> +
> +struct interp *
> +interp_lookup (const char *name)
> +{
> +  struct ui *ui = current_ui;
> +  struct interp_factory *factory;
> +  struct interp *interp;
> +  int ix;
> +
> +  if (name == NULL || strlen (name) == 0)
> +    return NULL;
> +
> +  /* Only create each interpreter once per top level.  */
> +  interp = interp_lookup_existing (name);
> +  if (interp != NULL)
> +    return interp;
> +
> +  for (ix = 0;
> +       VEC_iterate (interp_factory_p, interpreter_factories, ix, factory);
> +       ++ix)
> +    if (strcmp (factory->name, name) == 0)
> +      {
> +	interp = factory->func (name, ui);
> +	interp_add (interp);
> +	return interp;
> +      }
> +
> +  return NULL;
> +}
> +

I think there are some opportunities to reduce the number of functions that directly
access global state.  For example, I think it would be clearer if interp_lookup took
a struct ui* as argument instead of accessing current_ui.  It would show that it's used
to lookup an interpreter by name in a ui.  Similarly, it calls interp_lookup_existing and
interp_add, which both use get_current_interp_info(), which is essentially accessing
current_ui.  I believe it would be easier to follow if some caller provided the struct ui
and it was passed down by parameters.


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