Re: [PATCH v3 2/3] Packet sniffer core framework

From
David Miller <davem@davemloft.net>
Date
2015-02-23 21:37:22
Also in
linux-devicetree, lkml
Reply to
Stathis Voukelatos, 2015-02-23
From: Stathis Voukelatos <redacted>
Date: Mon, 23 Feb 2015 14:26:21 +0000
quoted hunk ↗ jump to hunk
+	spin_lock_irqsave(&priv->lock, flags);
+	/* Stop the hardware */
+	sch->stop(sch);
+	/* Set the new command pattern */
+	ret = sch->set_pattern(sch, skb->data, skb->len / 2);
+	/* Restart the hardware */
+	sch->start(sch);
+	spin_unlock_irqrestore(&priv->lock, flags);
These comments are excessive.

When someone calls ops->stop() what are they supposed to think the
thing does?  Open a can of tuna?  Mow the lawn?  Wash the dishes?  No,
it stops the thing.  Everyone understands that and you don't have to
explicitly say it.

Saying "stop the hardware" does not add anything to the source code
that is not already implicitly there.  They just take up space and
keep more useful information from being displayed at once on the
screen.  Please remove all of these things.

Thanks.
Thread (8 messages) 8 messages, 2 authors, last reply 2015-02-24
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help