linux-kernel - Re: [PATCH] vt6656: Coding style fixes

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] [day] [month] [year] [list]

Message-ID: <1493914131.22125.35.camel@perches.com>
Date:   Thu, 04 May 2017 09:08:51 -0700
From:   Joe Perches <joe@...ches.com>
To:     Simo Koskinen <koskisoft@...il.com>, forest@...ttletooquiet.net
Cc:     gregkh@...uxfoundation.org, devel@...verdev.osuosl.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vt6656: Coding style fixes

On Thu, 2017-05-04 at 17:33 +0200, Simo Koskinen wrote:
> Fixed coding style warnings reported by checkpatch.pl.

Please strive to do more than shut up checkpatch.

Think a little about what this code is doing.
Spend the time to analyze the code and improve it.

For instance, this function is currently:

static u32 vnt_get_rsvtime(struct vnt_private *priv, u8 pkt_type,
			    u32 frame_length, u16 rate, int need_ack)
{
	u32 data_time, ack_time;

	data_time = vnt_get_frame_time(priv->preamble_type, pkt_type,
				       frame_length, rate);

	if (pkt_type == PK_TYPE_11B)
		ack_time = vnt_get_frame_time(priv->preamble_type, pkt_type,
					      14, (u16)priv->top_cck_basic_rate);
	else
		ack_time = vnt_get_frame_time(priv->preamble_type, pkt_type,
					      14, (u16)priv->top_ofdm_basic_rate);

	if (need_ack)
		return data_time + priv->sifs + ack_time;

	return data_time;
}

It is computing how long it takes to do something.

It also is doing the ack_time calculation unnecessarily
when need_ack is not set.

Better code might be something like:

static u32 vnt_get_rsvtime(struct vnt_private *priv, u8 pkt_type,
			   u32 frame_length, u16 rate, int need_ack)
{
	u32 data_time, ack_time;

	data_time = vnt_get_frame_time(priv->preamble_type, pkt_type,
				       frame_length, rate);

	if (!need_ack)
		return data_time;

	if (pkt_type == PK_TYPE_11B)
		rate = priv->top_cck_basic_rate;
	else
		rate = priv->top_ofdm_basic_rate;

	ack_time = vnt_get_frame_time(priv->preamble_type, pkt_type, 14, rate);

	return data_time + priv->sifs + ack_time;
}

where the return of data_time is done when !need_ack
and rate is calculated and a single vnt_get_frame_time
call is used for ack_time only when necessary.

It's slightly smaller object code and faster to execute
when need_ack is not set.

Ideally the hard-coded 14 value in the ack_time calculation
should be a #define or a sizeof.