On Wed, Oct 30, 2013 at 2:56 AM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
> 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.carpenter@xxxxxxxxxx> 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.
I know I didn't have to respond back to this, but I wanted to let you
know that I like those free bonus comments :)
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
