On Wed, Oct 30, 2013 at 02:51:45AM -0700, Lisa Nguyen wrote:
> On Wed, Oct 30, 2013 at 2:01 AM, Dan Carpenter <dan.carpen...@oracle.com> 
> wrote:
> > On Tue, Oct 29, 2013 at 10:06:34PM -0700, Lisa Nguyen wrote:
> >> --- a/drivers/staging/bcm/Bcmchar.c
> >> +++ b/drivers/staging/bcm/Bcmchar.c
> >> @@ -287,9 +287,9 @@ static long bcm_char_ioctl(struct file *filp, UINT 
> >> cmd, ULONG arg)
> >>               struct bcm_rdm_buffer sRdmBuffer = {0};
> >>               PCHAR temp_buff = NULL;
> >>               UINT uiTempVar = 0;
> >> -             if ((Adapter->IdleMode == TRUE) ||
> >> -                     (Adapter->bShutStatus == TRUE) ||
> >> -                     (Adapter->bPreparingForLowPowerMode == TRUE)) {
> >> +             if ((Adapter->IdleMode == true) ||
> >> +                     (Adapter->bShutStatus == true) ||
> >> +                     (Adapter->bPreparingForLowPowerMode == true)) {
> >
> > Your patch is fine but these variable names suck.  How is "Mode" or
> > "Status" true or false?  It should be something like:
> >
> >                 if (adapter->idle || adapter->shutdown ||
> >                     addapter->preparing_for_low_power) {
> >
> > (I assume that's what the code is trying to say).
> 
> A big chunk of the bcm driver code is ugly to begin with.
> 
> I was focused on cleaning up pointless typedefs before attempting to
> rewrite the conditionals as they are an eyesore. I haven't read too
> much into the code yet.
> 

Yes.  Yes.  I understand.  These are just bonus comments I throw out for
free.  They don't require a response.

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

Reply via email to