| Message ID | 1305479060-22755-1-git-send-email-cnoss@gmx.de (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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 <cnoss@gmx.de> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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<cnoss@gmx.de> >> --- > 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
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<cnoss@gmx.de> > --- Hi Clemens, Please note that patches on our driver should be sent to Greg KH <gregkh@suse.de> and Cc <devel@linuxdriverproject.org> as we are still in staging tree. Gr. AvS
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++)
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 <cnoss@gmx.de> --- 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(-)