Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Pedro Alves <palves@redhat.com>
To: Philipp Rudo <prudo@linux.vnet.ibm.com>, Yao Qi <qiyaoltc@gmail.com>
Cc: gdb-patches@sourceware.org, omair.javaid@linaro.org,
	yao.qi@linaro.org, peter.griffin@linaro.org,
	arnez@linux.vnet.ibm.com
Subject: Re: [RFC v4 3/9] Add basic Linux kernel support
Date: Fri, 16 Jun 2017 11:43:00 -0000	[thread overview]
Message-ID: <f5b2cd2e-edeb-1bb0-fcdb-1d15562f1afa@redhat.com> (raw)
In-Reply-To: <20170616121026.024f664b@ThinkPad>

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,
Pedro Alves


  reply	other threads:[~2017-06-16 11:43 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-12 17:08 [RFC v4 0/9] Support for Linux kernel debugging Philipp Rudo
2017-06-12 17:08 ` [RFC v4 2/9] Add libiberty/concat styled concat_path function Philipp Rudo
2017-06-12 17:08 ` [RFC v4 7/9] Add privileged registers for s390x Philipp Rudo
2017-06-19 13:34   ` Yao Qi
2017-06-19 15:46     ` Philipp Rudo
2017-06-12 17:08 ` [RFC v4 1/9] Convert substitute_path_component to C++ Philipp Rudo
2017-06-12 17:08 ` [RFC v4 3/9] Add basic Linux kernel support Philipp Rudo
2017-06-15 15:23   ` Yao Qi
2017-06-16 10:10     ` Philipp Rudo
2017-06-16 11:43       ` Pedro Alves [this message]
2017-06-19  7:56         ` Philipp Rudo
2017-06-19  9:52       ` Yao Qi
2017-06-19 13:35         ` Omair Javaid
2017-06-19 15:44         ` Philipp Rudo
2017-06-12 17:08 ` [RFC v4 4/9] Add kernel module support for linux-kernel target Philipp Rudo
2017-06-12 17:08 ` [RFC v4 5/9] Add commands " Philipp Rudo
2017-06-12 17:08 ` [RFC v4 8/9] Link frame_info to thread_info Philipp Rudo
2017-06-19 13:07   ` Yao Qi
2017-06-19 15:46     ` Philipp Rudo
2017-06-12 17:09 ` [RFC v4 6/9] Separate common s390-tdep.* from s390-linux-tdep.* Philipp Rudo
2017-06-12 17:39 ` [RFC v4 9/9] Add S390 support for linux-kernel target Philipp Rudo
2017-06-19 13:20   ` Yao Qi
2017-06-19 15:45     ` Philipp Rudo
2017-06-12 21:17 ` Philipp Rudo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f5b2cd2e-edeb-1bb0-fcdb-1d15562f1afa@redhat.com \
    --to=palves@redhat.com \
    --cc=arnez@linux.vnet.ibm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=omair.javaid@linaro.org \
    --cc=peter.griffin@linaro.org \
    --cc=prudo@linux.vnet.ibm.com \
    --cc=qiyaoltc@gmail.com \
    --cc=yao.qi@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox