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

[Philipp Rudo <prudo at linux dot vnet dot ibm dot com>]

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