netdev - Re: [PATCH 04/32] staging: wfx: remove "burst" mechanism

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: <2993281.fr9RtUPQsq@pc-42>
Date:   Thu, 2 Apr 2020 14:47:45 +0000
From:   Jérôme Pouiller <Jerome.Pouiller@...abs.com>
To:     Dan Carpenter <dan.carpenter@...cle.com>
CC:     "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "David S . Miller" <davem@...emloft.net>,
        Kalle Valo <kvalo@...eaurora.org>
Subject: Re: [PATCH 04/32] staging: wfx: remove "burst" mechanism

On Thursday 2 April 2020 15:05:26 CEST Dan Carpenter wrote:
[...]
>                             ^^^^^^^^^
> Not related to this this patch but this confused me initially.  UINT_MAX
> would be more readable.
> 
> The other unrelated question I had about this function was:
> 
>    402          /* search for a winner using edca params */
>    403          for (i = 0; i < IEEE80211_NUM_ACS; ++i) {
>                                 ^^^^^^^^^^^^^^^^^
> IEEE80211_NUM_ACS is 4.
> 
>    404                  int queued;
>    405
>    406                  edca = &wvif->edca_params[i];
>    407                  queued = wfx_tx_queue_get_num_queued(&wvif->wdev->tx_queue[i],
>    408                                  tx_allowed_mask);
>    409                  if (!queued)
>    410                          continue;
>    411                  *total += queued;
>    412                  score = ((edca->aifs + edca->cw_min) << 16) +
>    413                          ((edca->cw_max - edca->cw_min) *
>    414                           (get_random_int() & 0xFFFF));
>    415                  if (score < best && (winner < 0 || i != 3)) {
>                                                            ^^^^^^
> 
> Why do we not want winner to be 3?  It's unrelated to the patch but
> there should be a comment next to that code probably.
> 
>    416                          best = score;
>    417                          winner = i;
>    418                  }
>    419          }

Indeed. In add, this code is useless. That's why I drop this code in
patch 22/32.

-- 
Jérôme Pouiller