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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20150223.163717.356301805782993412.davem@davemloft.net>
Date:	Mon, 23 Feb 2015 16:37:17 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	stathis.voukelatos@...n.co.uk
Cc:	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	devicetree@...r.kernel.org
Subject: Re: [PATCH v3 2/3] Packet sniffer core framework

From: Stathis Voukelatos <stathis.voukelatos@...n.co.uk>
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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html