Re: [patch] staging: panel: pass correct lengths to keypad_send_key()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Am 02.12.2012 12:53, schrieb Willy Tarreau:
> Hi Dan,
> 
> On Sun, Dec 02, 2012 at 02:22:13PM +0300, Dan Carpenter wrote:
>> On Fri, Nov 30, 2012 at 10:19:06AM +0100, walter harms wrote:
>>>>  				if (press_str[0])
>>>>  					keypad_send_key(press_str,
>>>> -							sizeof(press_str));
>>>> +							sizeof(input->u.kbd.press_str));
>>>>  			}
>>> yep, this looks like the original intention.
>>> NTL i would like to question the use of  press_str and friends.
>>> Moving the if (press_str[0]) into keypad_send_key() seems
>>> more sensible.
>>>
>>> just my two cents,
>>>
>>
>> Greg had already applied my patch by the you sent this email.
>>
>> What you're saying sounds like it should be a good cleanup, but when
>> I actually look at it, the callers still have to test repeat_str[0]
>> so it's not as useful as one would hope.
>>
>> Really this stuff is really confusing.  I'm not sure what is
>> actually stored in press_str.  Is it a pointer or just chars?  If
>> it's a pointer then why are we only checking the first bit instead
>> of checking the pointer?  I think it's characters but why is it
>> declared as "char press_str[sizeof(void *) + sizeof(int)];"?
> 
> It's just chars. It was declared this way so that it did not
> inflate the union it's declared in. This is totally ugly and
> should be cleaned. But it dates 13 years ago when we were trying
> to shrink everything to have full-featured kernels in less than
> 500kB to boot from floppies...
> 

It is not a bad think to save space.
The question is about readability.

re,
 wh

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


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux