All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave <kilroyd@googlemail.com>
To: Walter Goldens <goldenstranger@yahoo.com>
Cc: "John W. Linville" <linville@tuxdriver.com>,
	linux-wireless@vger.kernel.org
Subject: Re: [PATCH] fix several minor description typos
Date: Mon, 17 May 2010 22:49:30 +0100	[thread overview]
Message-ID: <4BF1B9EA.8060009@gmail.com> (raw)
In-Reply-To: <974011.51480.qm@web56808.mail.re3.yahoo.com>

You may want to prefix the patch title with 'wireless:', and include in
the commitlog a description what type of errors you are correcting. In
this case I'd guess 'avoid word repetition'.

Most of the changes seem obviously correct. However a couple:

On 17/05/10 20:33, Walter Goldens wrote:
> --- a/drivers/net/wireless/orinoco/main.h
> +++ b/drivers/net/wireless/orinoco/main.h
> @@ -12,7 +12,7 @@
>  /* Compile time configuration and compatibility stuff               */
>  /********************************************************************/
>  
> -/* We do this this way to avoid ifdefs in the actual code */
> +/* We do that this way to avoid ifdefs in the actual code */
>  #ifdef WIRELESS_SPY
>  #define SPY_NUMBER(priv)    (priv->spy_data.spy_number)
>  #else

The latter construction doesn't make sense (to me). The former does,
even if it is a little clumsy. If the comment is still useful, I suggest
rewording it as something like:

/* Define SPY_NUMBER like this to avoid ifdefs in the actual code */

> --- a/include/linux/ieee80211.h
> +++ b/include/linux/ieee80211.h
> @@ -302,7 +302,7 @@ static inline int ieee80211_is_data_qos(
>  static inline int ieee80211_is_data_present(__le16 fc)
>  {
>      /*
> -     * mask with 0x40 and test that that bit is clear to only return true
> +     * mask with 0x40 and test that this bit is clear to only return true
>       * for the data-containing substypes.
>       */
>      return (fc & cpu_to_le16(IEEE80211_FCTL_FTYPE | 0x40)) ==

You may also want to correct the spelling of 'subtypes' here. I also
think that this could be reworded better. After consulting
IEE802.11-2007, section 7.1.3.1.2, I'd suggest:

/* Only return true for the data-containing subtypes.
 * Bit 0x40 indicates the frame subtype does not contain a Frame Body
 * field (i.e. there is no data). This bit should not be set.
 */

But I'd confirm with one of the core wireless developers before making
that change. It may also be worth changing the code to use
IEEE80211_STYPE_NULLFUNC instead of 0x40.


Regards,

Dave.

  reply	other threads:[~2010-05-17 21:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-05-17 19:33 [PATCH] fix several minor description typos Walter Goldens
2010-05-17 21:49 ` Dave [this message]
2010-05-18 11:44   ` [PATCH v1] wireless: " Walter Goldens

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4BF1B9EA.8060009@gmail.com \
    --to=kilroyd@googlemail.com \
    --cc=goldenstranger@yahoo.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=linville@tuxdriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.