Re: [RFC v4 3/9] Add basic Linux kernel support

Philipp Rudo
2017-06-19T07:56:01Z

Hi Pedro

On Fri, 16 Jun 2017 12:43:02 +0100
Pedro Alves <palves@redhat.com> wrote:

> On 06/16/2017 11:10 AM, Philipp Rudo wrote:
> > True, but that would lead to code duplication. That's why I chose the way
> > using the two goto.
> >
> > Without goto the function reads like this
> >
> > struct lk_private_data
> > * lk_init_struct (const char *name, const char *alias, int
> > silent) {
>
>
> > {
> > /* Chek for "typedef struct { ... } name;"-like definitions.
> > */ sym = lookup_symbol (name, global, VAR_DOMAIN,
> > NULL).symbol; if (sym ==
> > NULL)
> > { if (!silent)
> > error (_("Could not find %s. Aborting."),
> > alias);
> > return
> > NULL; }
> >
> > type = check_typedef (SYMBOL_TYPE
> > (sym)); if (TYPE_CODE (type) !=
> > TYPE_CODE_STRUCT)
> > { if (!silent)
> > error (_("Could not find %s. Aborting."),
> > alias);
> > return
> > NULL; }
> > }
>
> You could add a helper function or lambda that handles the
> error return. E.g., with a lambda it'd look something like:
>
> lk_init_struct (const char *name, const char *alias, bool
> silent)
> { ...
> auto not_found = [=] ()
> {
> if (!silent)
> error (_("Could not find %s. Aborting."), alias);
> return NULL;
> };
> ...
> sym = lookup_symbol (name, global, VAR_DOMAIN,
> NULL).symbol; if (sym == NULL)
> return not_found ();
>
> type = check_typedef (SYMBOL_TYPE
> (sym)); if (TYPE_CODE (type) !=
> TYPE_CODE_STRUCT) return not_found ();
>
>
> Alternatively, since NULL return is always an error
> indication, split into two functions, one that returns
> NULL if not found, and another that throws on error.
> The latter calls the former. I.e., something like this:
>
> /* Returns NULL if not found. */
> lk_private_data *
> lk_init_struct_nothrow (const char *name, const char *alias)
> {
> ...
> sym = lookup_symbol (name, global, VAR_DOMAIN,
> NULL).symbol; if (sym == NULL)
> return NULL;
> ...
> return data;
> }
>
> /* Either returns non-NULL, or throws an error. */
>
> lk_private_data *
> lk_init_struct (const char *name, const char *alias)
> {
> lk_private_data *data = lk_init_struct_nothrow (name, alias);
> if (data == NULL)
> error (_("Could not find %s. Aborting."), alias);
> return data;
> }

Thanks for the tip. As I still live in a C world, I prefer your second
approach. The code now look like this

/* Helper function for lk_init_struct. Returns NULL if symbol could not be
found. Doesn't throw an error. */

struct lk_private_data *
lk_init_struct_silent (const char *name, const char *alias)
{
struct lk_private_data *data;
const struct block *global;
const struct symbol *sym;
struct type *type;
void **new_slot;
void *old_slot;

if ((old_slot = lk_find (alias)) != NULL)
return (struct lk_private_data *) old_slot;

global = block_global_block(get_selected_block (0));
sym = lookup_symbol (name, global, STRUCT_DOMAIN, NULL).symbol;

if (sym != NULL)
{
type = SYMBOL_TYPE (sym);
}
else
{
/* Chek for "typedef struct { ... } name;"-like definitions. */
sym = lookup_symbol (name, global, VAR_DOMAIN, NULL).symbol;
if (sym == NULL)
return NULL;

type = check_typedef (SYMBOL_TYPE (sym));
if (TYPE_CODE (type) != TYPE_CODE_STRUCT)
return NULL;
}

data = XCNEW (struct lk_private_data);
data->alias = alias;
data->data.type = type;

new_slot = lk_find_slot (alias);
*new_slot = data;

return data;
}

/* Same as lk_init_addr but for structs. */

struct lk_private_data *
lk_init_struct (const char *name, const char *alias, bool silent)
{
struct lk_private_data *data;
data = lk_init_struct_silent (name, alias);

if (data == NULL && !silent)
error (_("Could not find %s. Aborting."), alias);

return data;
}

Philipp