bcm_pktq_flush and related functions only ever get 0 or a pointer for
arg, so make it a pointer.
This might fix a crash on 64bit.
Signed-off-by: Clemens Noss <[email protected]>
---
drivers/staging/brcm80211/brcmfmac/dhd_sdio.c | 2 +-
drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c | 4 ++--
drivers/staging/brcm80211/brcmsmac/wlc_main.c | 4 ++--
drivers/staging/brcm80211/include/bcmutils.h | 6 +++---
drivers/staging/brcm80211/util/bcmutils.c | 4 ++--
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c b/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
index 8683eeb..a71c6f8 100644
--- a/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
+++ b/drivers/staging/brcm80211/brcmfmac/dhd_sdio.c
@@ -2845,7 +2845,7 @@ void dhd_bus_stop(struct dhd_bus *bus, bool enforce_mutex)
dhdsdio_clkctl(bus, CLK_SDONLY, false);
/* Clear the data packet queues */
- bcm_pktq_flush(&bus->txq, true, NULL, 0);
+ bcm_pktq_flush(&bus->txq, true, NULL, NULL);
/* Clear any held glomming stuff */
if (bus->glomd)
diff --git a/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c b/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
index 5b041a6..0d93bd6 100644
--- a/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
+++ b/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
@@ -1225,7 +1225,7 @@ void wlc_ampdu_shm_upd(struct ampdu_info *ampdu)
/*
* callback function that helps flushing ampdu packets from a priority queue
*/
-static bool cb_del_ampdu_pkt(void *p, int arg_a)
+static bool cb_del_ampdu_pkt(void *p, void *arg_a)
{
struct sk_buff *mpdu = (struct sk_buff *)p;
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(mpdu);
@@ -1269,7 +1269,7 @@ void wlc_ampdu_flush(struct wlc_info *wlc,
ampdu_pars.tid = tid;
for (prec = 0; prec < pq->num_prec; prec++) {
bcm_pktq_pflush(pq, prec, true, cb_del_ampdu_pkt,
- (int)&du_pars);
+ (void *)&du_pars);
}
wlc_inval_dma_pkts(wlc->hw, sta, dma_cb_fn_ampdu);
}
diff --git a/drivers/staging/brcm80211/brcmsmac/wlc_main.c b/drivers/staging/brcm80211/brcmsmac/wlc_main.c
index 2fed0eb..926b01a 100644
--- a/drivers/staging/brcm80211/brcmsmac/wlc_main.c
+++ b/drivers/staging/brcm80211/brcmsmac/wlc_main.c
@@ -2520,7 +2520,7 @@ uint wlc_down(struct wlc_info *wlc)
/* flush tx queues */
for (qi = wlc->tx_queues; qi != NULL; qi = qi->next) {
- bcm_pktq_flush(&qi->q, true, NULL, 0);
+ bcm_pktq_flush(&qi->q, true, NULL, NULL);
}
callbacks += wlc_bmac_down_finish(wlc->hw);
@@ -8135,7 +8135,7 @@ void wlc_wait_for_tx_completion(struct wlc_info *wlc, bool drop)
{
/* flush packet queue when requested */
if (drop)
- bcm_pktq_flush(&wlc->pkt_queue->q, false, NULL, 0);
+ bcm_pktq_flush(&wlc->pkt_queue->q, false, NULL, NULL);
/* wait for queue and DMA fifos to run dry */
while (!pktq_empty(&wlc->pkt_queue->q) ||
diff --git a/drivers/staging/brcm80211/include/bcmutils.h b/drivers/staging/brcm80211/include/bcmutils.h
index aa00e83..1d57934 100644
--- a/drivers/staging/brcm80211/include/bcmutils.h
+++ b/drivers/staging/brcm80211/include/bcmutils.h
@@ -74,7 +74,7 @@
#define PKTQ_PREC_ITER(pq, prec) for (prec = (pq)->num_prec - 1; prec >= 0; prec--)
/* fn(pkt, arg). return true if pkt belongs to if */
- typedef bool(*ifpkt_cb_t) (void *, int);
+ typedef bool(*ifpkt_cb_t) (void *, void *);
/* operations on a specific precedence in packet queue */
@@ -100,7 +100,7 @@ extern void bcm_pkt_buf_free_skb(struct sk_buff *skb);
/* Empty the queue at particular precedence level */
extern void bcm_pktq_pflush(struct pktq *pq, int prec,
- bool dir, ifpkt_cb_t fn, int arg);
+ bool dir, ifpkt_cb_t fn, void *arg);
/* operations on a set of precedences in packet queue */
@@ -127,7 +127,7 @@ extern void bcm_pktq_init(struct pktq *pq, int num_prec, int max_len);
/* prec_out may be NULL if caller is not interested in return value */
extern struct sk_buff *bcm_pktq_peek_tail(struct pktq *pq, int *prec_out);
extern void bcm_pktq_flush(struct pktq *pq, bool dir,
- ifpkt_cb_t fn, int arg);
+ ifpkt_cb_t fn, void *arg);
/* externs */
/* packet */
diff --git a/drivers/staging/brcm80211/util/bcmutils.c b/drivers/staging/brcm80211/util/bcmutils.c
index e185e28..43e5bb3 100644
--- a/drivers/staging/brcm80211/util/bcmutils.c
+++ b/drivers/staging/brcm80211/util/bcmutils.c
@@ -231,7 +231,7 @@ EXPORT_SYMBOL(bcm_pktq_pdeq_tail);
void
bcm_pktq_pflush(struct pktq *pq, int prec, bool dir,
- ifpkt_cb_t fn, int arg)
+ ifpkt_cb_t fn, void *arg)
{
struct pktq_prec *q;
struct sk_buff *p, *prev = NULL;
@@ -263,7 +263,7 @@ bcm_pktq_pflush(struct pktq *pq, int prec, bool dir,
EXPORT_SYMBOL(bcm_pktq_pflush);
void bcm_pktq_flush(struct pktq *pq, bool dir,
- ifpkt_cb_t fn, int arg)
+ ifpkt_cb_t fn, void *arg)
{
int prec;
for (prec = 0; prec < pq->num_prec; prec++)
--
1.7.4.5
to clarify the intention:
On Sun, May 15, 2011 at 09:54:26PM +0200, Arend van Spriel wrote:
> On 05/15/2011 08:10 PM, Hauke Mehrtens wrote:
[...]
>> Why don't you use the correct pointer type here instead of casting then
>> around and the parameter names are meaningless too.
>>
>> static bool cb_del_ampdu_pkt(struct sk_buff *mpdu, struct
>> cb_del_ampdu_pars *ampdu_pars)
[...]
> Actually this is a generic callback mechanism in which an additional
> parameter can be passed, which can be of *any* type hence a void pointer
> seems justified here and a meaningless name is used. I do agree that the
> txi parameter will always be a struct sk_buff and should be indicated as
> such.
Reported-by: Hauke Mehrtens <[email protected]>
Signed-off-by: Clemens Noss <[email protected]>
---
drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c | 3 +--
drivers/staging/brcm80211/include/bcmutils.h | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c b/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
index 0d93bd6..b1bc1a6 100644
--- a/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
+++ b/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
@@ -1225,9 +1225,8 @@ void wlc_ampdu_shm_upd(struct ampdu_info *ampdu)
/*
* callback function that helps flushing ampdu packets from a priority queue
*/
-static bool cb_del_ampdu_pkt(void *p, void *arg_a)
+static bool cb_del_ampdu_pkt(struct sk_buff *mpdu, void *arg_a)
{
- struct sk_buff *mpdu = (struct sk_buff *)p;
struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(mpdu);
struct cb_del_ampdu_pars *ampdu_pars =
(struct cb_del_ampdu_pars *)arg_a;
diff --git a/drivers/staging/brcm80211/include/bcmutils.h b/drivers/staging/brcm80211/include/bcmutils.h
index 1d57934..4dc190b 100644
--- a/drivers/staging/brcm80211/include/bcmutils.h
+++ b/drivers/staging/brcm80211/include/bcmutils.h
@@ -74,7 +74,7 @@
#define PKTQ_PREC_ITER(pq, prec) for (prec = (pq)->num_prec - 1; prec >= 0; prec--)
/* fn(pkt, arg). return true if pkt belongs to if */
- typedef bool(*ifpkt_cb_t) (void *, void *);
+typedef bool(*ifpkt_cb_t) (struct sk_buff *, void *);
/* operations on a specific precedence in packet queue */
--
1.7.4.5
On 05/15/2011 07:04 PM, Clemens Noss wrote:
> bcm_pktq_flush and related functions only ever get 0 or a pointer for
> arg, so make it a pointer.
>
> This might fix a crash on 64bit.
>
> Signed-off-by: Clemens Noss <[email protected]>
> ---
> drivers/staging/brcm80211/brcmfmac/dhd_sdio.c | 2 +-
> drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c | 4 ++--
> drivers/staging/brcm80211/brcmsmac/wlc_main.c | 4 ++--
> drivers/staging/brcm80211/include/bcmutils.h | 6 +++---
> drivers/staging/brcm80211/util/bcmutils.c | 4 ++--
> 5 files changed, 10 insertions(+), 10 deletions(-)
> ....
> diff --git a/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c b/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
> index 5b041a6..0d93bd6 100644
> --- a/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
> +++ b/drivers/staging/brcm80211/brcmsmac/wlc_ampdu.c
> @@ -1225,7 +1225,7 @@ void wlc_ampdu_shm_upd(struct ampdu_info *ampdu)
> /*
> * callback function that helps flushing ampdu packets from a priority queue
> */
> -static bool cb_del_ampdu_pkt(void *p, int arg_a)
> +static bool cb_del_ampdu_pkt(void *p, void *arg_a)
> {
> struct sk_buff *mpdu = (struct sk_buff *)p;
> struct ieee80211_tx_info *tx_info = IEEE80211_SKB_CB(mpdu);
Why don't you use the correct pointer type here instead of casting then
around and the parameter names are meaningless too.
static bool cb_del_ampdu_pkt(struct sk_buff *mpdu, struct
cb_del_ampdu_pars *ampdu_pars)
The signature of dma_cb_fn_ampdu(void *txi, void *arg_a) should be
changed too.
> ...
Hauke
On 05/15/2011 07:04 PM, Clemens Noss wrote:
> bcm_pktq_flush and related functions only ever get 0 or a pointer for
> arg, so make it a pointer.
>
> This might fix a crash on 64bit.
>
> Signed-off-by: Clemens Noss<[email protected]>
> ---
Hi Clemens,
Please note that patches on our driver should be sent to Greg KH
<[email protected]> and Cc <[email protected]> as we are still
in staging tree.
Gr. AvS
--
Almost nobody dances sober, unless they happen to be insane.
-- H.P. Lovecraft --
On 05/15/2011 08:10 PM, Hauke Mehrtens wrote:
> On 05/15/2011 07:04 PM, Clemens Noss wrote:
>> bcm_pktq_flush and related functions only ever get 0 or a pointer for
>> arg, so make it a pointer.
>>
>> This might fix a crash on 64bit.
>>
>> Signed-off-by: Clemens Noss<[email protected]>
>> ---
> Why don't you use the correct pointer type here instead of casting then
> around and the parameter names are meaningless too.
>
> static bool cb_del_ampdu_pkt(struct sk_buff *mpdu, struct
> cb_del_ampdu_pars *ampdu_pars)
>
> The signature of dma_cb_fn_ampdu(void *txi, void *arg_a) should be
> changed too.
Actually this is a generic callback mechanism in which an additional
parameter can be passed, which can be of *any* type hence a void pointer
seems justified here and a meaningless name is used. I do agree that the
txi parameter will always be a struct sk_buff and should be indicated as
such.
Apart from that: good catch. This indeed may cause issues on 64-bit systems.
Gr. AvS
--
Almost nobody dances sober, unless they happen to be insane.
-- H.P. Lovecraft --
> Reported-by: Hauke Mehrtens<[email protected]>
> Signed-off-by: Clemens Noss<[email protected]>
Acked-by: Roland Vossen <[email protected]>