On Wed, Nov 11, 2015 at 02:03:02PM +0000, Luick, Dean wrote:
> 
> 
> > -----Original Message-----
> > From: linux-rdma-ow...@vger.kernel.org [mailto:linux-rdma-
> > ow...@vger.kernel.org] On Behalf Of Dan Carpenter
> > Sent: Wednesday, November 11, 2015 2:45 AM
> > To: John, Jubin <jubin.j...@intel.com>
> > Cc: gre...@linuxfoundation.org; de...@driverdev.osuosl.org; linux-
> > r...@vger.kernel.org; dledf...@redhat.com
> > Subject: Re: [PATCH 12/13] staging/rdma/hfi1: Read EFI variable for device
> > description
> > 
> > On Wed, Nov 11, 2015 at 02:33:32AM -0500, Jubin John wrote:
> > > +static int read_efi_var(const char *name, unsigned long *size,
> > > +                 void **return_data)
> > > +{
> > > + int ret;
> > > +
> > > + /* set failure return values */
> > > + *size = 0;
> > > + *return_data = NULL;
> > > +
> > > + /*
> > > +  * Use EFI run-time support to obtain an EFI variable.  Support may
> > > +  * be compiled out, so declare all variables inside.
> > > +  */
> > > + if (efi_enabled(EFI_RUNTIME_SERVICES)) {
> > 
> > 
> > Flip this around:
> > 
> >     if (!efi_enabled(EFI_RUNTIME_SERVICES))
> >             return -ENOSYS;
> 
> The style here is very deliberate.
> 
> The issue is how efi_enabled() is defined via CONFIG options.
>  The function can be turned into a 0 if certain CONFIG variables are
> not set.  The code is structured to make all of the dependent
> variables disappear if efi_enabled() becomes 0.

This all understand.

>  If the code is shifted as you suggest, we will get builds from the
> automatic builders that try all combinations with unused variables.
>  This was done to avoid that.

I'm not sure I understand.  You are doing this to try tricking the
autobuilders into not testind certain configs?  What?  I don't
understand what you mean by unused variables.  There shouldn't be any
unused variable warnings.  If you are getting unused variable warnings
can you post one so that I can take a look?

Maybe you are worried the function is a waste of memory if you declare
the variables earlier before the if enabled check?  It's not a problem.
The compiler is smart enough to see the immediate return and removes
the dead code.

Perhaps I am not seeing your concern.

regards,
dan carpenter

_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to