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