[PATCH 12/13] staging/rdma/hfi1: Read EFI variable for device description

[gregkh at linuxfoundation.org]

On Wed, Nov 11, 2015 at 03:03:12PM +0000, Luick, Dean wrote:
> > -----Original Message-----
> > From: Dan Carpenter [mailto:dan.carpenter at oracle.com]
> > Sent: Wednesday, November 11, 2015 8:39 AM
> > To: Luick, Dean <dean.luick at intel.com>
> > Cc: John, Jubin <jubin.john at intel.com>; devel at driverdev.osuosl.org;
> > gregkh at linuxfoundation.org; dledford at redhat.com; linux-
> > rdma at vger.kernel.org
> > Subject: Re: [PATCH 12/13] staging/rdma/hfi1: Read EFI variable for device
> > description
> > 
> > > > > +	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?
> 
> Certainly not.  I did not explain this well.
> 
> > 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?
> 
> If you move the variables to the top and have the early return as you suggest, then in some CONFIG cases, there will be all those automatic variables declared but they are never used - the compiler has short-circuited the rest of the function.  Will not the compiler complain about unused variables in those cases?  That is the situation I was trying to avoid.

Try it and see (hint, I don't think so...)