Hi, I picked (void*) to be conformant with the other examples in htt_rx.c
For example at line 1431:
> rxd = HTT_RX_BUF_TO_RX_DESC(hw,
> (void *)msdu->data - hw->rx_desc_ops->rx_desc_size);
But for me it is ok. Maybe we should fix all the occurrences of this kind.
Greetings,
FM
Il giorno mar 22 feb 2022 alle ore 21:52 Jeff Johnson
<quic_jjohnson@xxxxxxxxxxx> ha scritto:
>
> On 2/21/2022 4:26 AM, Francesco Magliocca wrote:
> > Reading through the commit history, it looks like
> > there is no special need why we must skip the first 4 bytes
> > in this trace call:
> >
> > trace_ath10k_htt_rx_desc(ar, (void*)rx_desc + sizeof(u32),
> > hw->rx_desc_ops->rx_desc_size - sizeof(u32));
> >
> > found in the function ath10k_htt_rx_amsdu_pop in the file htt_rx.c
> >
> > i think the original author
> > (who is also the one who added rx_desc tracing capabilities
> > in a0883cf7e75a) just wanted to trace the rx_desc contents,
> > ignoring the fw_rx_desc_base info field
> > (which is the part being skipped over).
> > But the trace_ath10k_htt_rx_desc later added
> > don't care about skipping it, so it may be good
> > to uniform this call to the others in the file.
> > But this would change the output of the trace and
> > thus it may be a problem for tools that rely on it.
> > Therefore I propose until further discussion
> > to just keep it as it is and just fix the pointer arithmetic bug.
> >
> > Add missing void* cast to rx descriptor pointer in order to
> > properly skip the initial 4 bytes of the rx descriptor
> > when passing it to trace_ath10k_htt_rx_desc trace function.
> >
> > This fixes the pointer arithmetic error detected
> > by Dan Carpenter's static analysis tool.
> >
> > Fixes: 6bae9de622d3 ("ath10k: abstract htt_rx_desc structure")
> >
> > Tested-on: QCA6174 hw3.2 PCI WLAN.RM.4.4.1-00157-QCARMSWPZ-1
> >
> > Signed-off-by: Francesco Magliocca <franciman12@xxxxxxxxx>
> > Link: https://lore.kernel.org/ath10k/20220201130900.GD22458@kili/
> > ---
> > drivers/net/wireless/ath/ath10k/htt_rx.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> > index 9ad64ca84beb..e01efcd2ce06 100644
> > --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> > +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> > @@ -429,7 +429,7 @@ static int ath10k_htt_rx_amsdu_pop(struct ath10k_htt *htt,
> > RX_MSDU_END_INFO0_LAST_MSDU;
> >
> > /* FIXME: why are we skipping the first part of the rx_desc? */
> > - trace_ath10k_htt_rx_desc(ar, rx_desc + sizeof(u32),
> > + trace_ath10k_htt_rx_desc(ar, (void*)rx_desc + sizeof(u32),
>
> since void pointer arithmetic is undefined in C99 would it be "better"
> to cast as u8 *? I realize that gcc has an extension to support this
> [1], but this usage will cause a warning when -Wpointer-arith is used.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Pointer-Arith.html
