netdev - Re: [PATCH iwl-next v1] idpf: remove unreachable code from setting mailbox

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: <20250411104432.GZ395307@horms.kernel.org>
Date: Fri, 11 Apr 2025 11:44:32 +0100
From: Simon Horman <horms@...nel.org>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Cc: intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
	Aleksandr Loktionov <aleksandr.loktionov@...el.com>,
	Michal Kubiak <michal.kubiak@...el.com>,
	Pavan Kumar Linga <pavan.kumar.linga@...el.com>
Subject: Re: [PATCH iwl-next v1] idpf: remove unreachable code from setting
 mailbox

On Wed, Apr 09, 2025 at 08:29:45AM +0200, Michal Swiatkowski wrote:
> Remove code that isn't reached. There is no need to check for
> adapter->req_vec_chunks, because if it isn't set idpf_set_mb_vec_id()
> won't be called.
> 
> Only one path when idpf_set_mb_vec_id() is called:
> idpf_intr_req()
>  -> idpf_send_alloc_vectors_msg() -> adapter->req_vec_chunk is allocated
>  here, otherwise an error is returned and idpf_intr_req() exits with an
>  error.

I agree this is correct, but perhaps it would be clearer to say something
like this:

* idpf_set_mb_vec_id() is only called from idpf_intr_req()
* Before that idpf_intr_req() calls idpf_send_alloc_vectors_msg()
* idpf_send_alloc_vectors_msg() allocates adapter->req_vec_chunk

> 
> The idpf_set_mb_vec_id() becomes one-linear and it is called only once.

nit: one liner

> Remove it and set mailbox vector index directly.
> 
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
> Reviewed-by: Michal Kubiak <michal.kubiak@...el.com>
> Reviewed-by: Pavan Kumar Linga <pavan.kumar.linga@...el.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>

The above notwithstanding, this looks good to me.

Reviewed-by: Simon Horman <horms@...nel.org>