[PATCH 01/03] staging: dgap: fix a few more 80+ lines as reported by checkpatch

[Mark Hounschell]

On 03/05/2014 04:39 PM, Dan Carpenter wrote:
> Btw, if you don't get any messages from me that means I have given your
> patch the stamp of approval.  So good job on your previous patchset.  :)
>
> On Wed, Mar 05, 2014 at 03:54:49PM -0500, Mark Hounschell wrote:
>> @@ -1613,7 +1616,8 @@ static void dgap_tty_uninit(struct board_t *brd)
>>    * dgap_sniff - Dump data out to the "sniff" buffer if the
>>    * proc sniff file is opened...
>>    */
>> -static void dgap_sniff_nowait_nolock(struct channel_t *ch, uchar *text, uchar *buf, int len)
>> +static void dgap_sniff_nowait_nolock(struct channel_t *ch, uchar *text,
>> +					uchar *buf, int len)
>
> These don't line up properly.  Here is what it looks like:
>
> static void dgap_sniff_nowait_nolock(struct channel_t *ch, uchar *text,
> 					uchar *buf, int len)
>
>
> This is what it should look like:
>
> static void dgap_sniff_nowait_nolock(struct channel_t *ch, uchar *text,
> 				     uchar *buf, int len)
>
> [tab][tab][tab][tab][space][space][space][space][space]uchar *buf...
>
>
>> @@ -1686,7 +1692,8 @@ static void dgap_sniff_nowait_nolock(struct channel_t *ch, uchar *text, uchar *b
>>   			r = SNIFF_MAX - ch->ch_sniff_in;
>>
>>   			if (r <= n) {
>> -				memcpy(ch->ch_sniff_buf + ch->ch_sniff_in, p, r);
>> +				memcpy(ch->ch_sniff_buf + ch->ch_sniff_in,
>> +					p, r);
>
>
> Function arguments line up:
>
> 				memcpy(ch->ch_sniff_buf + ch->ch_sniff_in, p,
> 				       r);
>

As you say below, "Breaking the lines up like this isn't ideal". This 
one I feel should have been left as is. It seems to me that breaking the 
line like above should only be done, like if r was 8 chars or more? Even 
if p was also on the other side of col 80, I would normally leave p and 
r both alone?

>> @@ -2255,7 +2265,8 @@ static int dgap_block_til_ready(struct tty_struct *tty, struct file *file, struc
>>   		 * touched safely, the close routine will signal the
>>   		 * ch_wait_flags to wake us back up.
>>   		 */
>> -		if (!((ch->ch_tun.un_flags | ch->ch_pun.un_flags) & UN_CLOSING)) {
>> +		if (!((ch->ch_tun.un_flags | ch->ch_pun.un_flags) &
>> +			UN_CLOSING)) {
>
>
> This one lines up like this:
>
> 		if (!((ch->ch_tun.un_flags | ch->ch_pun.un_flags) &
> 		      UN_CLOSING)) {
>
> With the 'U' and third '(' on the same column.
>

OK, I think I got it. That's how I do it in my own code too. So should 
be easy to remember for staging.

> Breaking the lines up like this isn't ideal, of course.  I would be
> tempted to leave the code as-is.  In the end, we always apply
> checkpatch.pl fixes to stop people from resending them over and over but
> it's not the king of the world.
>

OK. Do ya think my rule above would be OK for me in this driver?

>> @@ -2431,7 +2444,8 @@ static void dgap_tty_close(struct tty_struct *tty, struct file *file)
>>   	 * Only officially close channel if count is 0 and
>>   	 * DIGI_PRINTER bit is not set.
>>   	 */
>> -	if ((ch->ch_open_count == 0) && !(ch->ch_digi.digi_flags & DIGI_PRINTER)) {
>> +	if ((ch->ch_open_count == 0) &&
>> +		!(ch->ch_digi.digi_flags & DIGI_PRINTER)) {
>>
>>   		ch->ch_flags &= ~(CH_RXBLOCK);
>
> On this one breaking the live up doesn't hurt readabilty at all.  It
> should look like this.
>
> 	if ((ch->ch_open_count == 0) &&
> 	    !(ch->ch_digi.digi_flags & DIGI_PRINTER)) {
>
> 		ch->ch_flags &= ~(CH_RXBLOCK);
>
> That way it's more cear that "ch->ch_flags &= ~(CH_RXBLOCK);" is not
> at the same indent level as "!(ch->ch_digi.digi_flags & DIGI_PRINTER))"
>

Right. Got it.

Thanks
Mark