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

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

 






On 23/02/15 21:37, David Miller wrote:
From: Stathis Voukelatos <stathis.voukelatos@xxxxxxxxxx>
Date: Mon, 23 Feb 2015 14:26:21 +0000

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


Will remove the comments in the next version of the patch set.

Thank you,
Stathis
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux