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
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to