Search Linux Wireless

Re: [PATCH] b43: fix regression in PIO case

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/27/2011 06:00 PM, Guennadi Liakhovetski wrote:
On Tue, 27 Dec 2011, Larry Finger wrote:

On 12/27/2011 05:05 PM, Guennadi Liakhovetski wrote:
On Tue, 27 Dec 2011, Rafał Miłecki wrote:

W dniu 26 grudnia 2011 18:28 użytkownik Guennadi Liakhovetski
<g.liakhovetski@xxxxxx>   napisał:
This patch fixes the regression, introduced by

commit 17030f48e31adde5b043741c91ba143f5f7db0fd
From: Rafał Miłecki<zajec5@xxxxxxxxx>
Date: Thu, 11 Aug 2011 17:16:27 +0200
Subject: [PATCH] b43: support new RX header, noticed to be used in
598.314+ fw

in PIO case.

Signed-off-by: Guennadi Liakhovetski<g.liakhovetski@xxxxxx>
---
diff --git a/drivers/net/wireless/b43/pio.c
b/drivers/net/wireless/b43/pio.c
index ce8a4bd..b64b64c 100644
--- a/drivers/net/wireless/b43/pio.c
+++ b/drivers/net/wireless/b43/pio.c
@@ -617,9 +617,19 @@ static bool pio_rx_frame(struct b43_pio_rxqueue *q)
         const char *err_msg = NULL;
         struct b43_rxhdr_fw4 *rxhdr =
                 (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
+       size_t rxhdr_size = sizeof(*rxhdr);

         BUILD_BUG_ON(sizeof(wl->pio_scratchspace)<   sizeof(*rxhdr));
-       memset(rxhdr, 0, sizeof(*rxhdr));
+       switch (dev->fw.hdr_format) {
+       case B43_FW_HDR_410:
+       case B43_FW_HDR_351:
+               rxhdr_size -= sizeof(rxhdr->format_598) -
+                       sizeof(rxhdr->format_351);
+               break;
+       case B43_FW_HDR_598:
+               break;
+       }
+       memset(rxhdr, 0, rxhdr_size);

Huuh, that's really tricky. Can you just do "normal" conditions as
Larry suggested, please?

Sorry? I absolutely see nothing tricky there. Do you think this would look
"less tricky" to you:

	switch (dev->fw.hdr_format) {
	case B43_FW_HDR_410:
	case B43_FW_HDR_351:
		rxhdr_size = offsetof(struct b43_rxhdr_fw4,
					format_351) +
			sizeof(rxhdr_size->format_351);
		break;
	case B43_FW_HDR_598:
		rxhdr_size = sizeof(*rxhdr);
		break;
	}


How about this?

Index: wireless-testing-new/drivers/net/wireless/b43/pio.c
===================================================================
--- wireless-testing-new.orig/drivers/net/wireless/b43/pio.c
+++ wireless-testing-new/drivers/net/wireless/b43/pio.c
@@ -617,9 +617,20 @@ static bool pio_rx_frame(struct b43_pio_
         const char *err_msg = NULL;
         struct b43_rxhdr_fw4 *rxhdr =
                 (struct b43_rxhdr_fw4 *)wl->pio_scratchspace;
+       size_t rxhdr_size;

         BUILD_BUG_ON(sizeof(wl->pio_scratchspace)<  sizeof(*rxhdr));
-       memset(rxhdr, 0, sizeof(*rxhdr));
+       switch (dev->fw.hdr_format) {
+       case B43_FW_HDR_410:
+       case B43_FW_HDR_351:
+               rxhdr_size = sizeof(rxhdr->format_351);
+               break;
+       case B43_FW_HDR_598:
+       default:
+               rxhdr_size = sizeof(rxhdr->format_598);
+               break;
+       }
+       memset(rxhdr, 0, rxhdr_size);

         /* Check if we have data and wait for it to get ready. */
         if (q->rev>= 8) {

I am sorry, I'm either being blind and stupid or you're trying to do
something quite wrong there. struct b43_rxhdr_fw4 has a bunch of fields
first, then at the end it has a union of two fields: format_598 and
format_351, right? rxhdr is pointing at the struct itself. Before the
offending patch memset() used to clean the whole struct. Now in your above
version you calculate the size of one of those union fields and nullify
that many bytes from the _beginning_ of the whole struct.

I've seen myself being wrong before, but here... I'll let you judge
though.

No, you are right. I misread the code. Your patch above would work and is probably as clean as one can expect.

Larry


--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux