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

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

 



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);

> @@ -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.

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.

> @@ -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))"

regards,
dan carpenter

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-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