This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v3 06/34] Introduce interpreter factories
- From: Simon Marchi <simon dot marchi at ericsson dot com>
- To: Pedro Alves <palves at redhat dot com>, <gdb-patches at sourceware dot org>
- Date: Wed, 18 May 2016 15:18:00 -0400
- Subject: Re: [PATCH v3 06/34] Introduce interpreter factories
- Authentication-results: sourceware.org; auth=none
- References: <1462538104-19109-1-git-send-email-palves at redhat dot com> <1462538104-19109-7-git-send-email-palves at redhat dot com>
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.