All of lore.kernel.org
 help / color / mirror / Atom feed
* [ath9k-devel] signed vs unsigned bug in ath9k
@ 2009-12-07 11:07 Dan Carpenter
  2009-12-07 15:55 ` Luis R. Rodriguez
  0 siblings, 1 reply; 8+ messages in thread
From: Dan Carpenter @ 2009-12-07 11:07 UTC (permalink / raw)
  To: ath9k-devel

drivers/net/wireless/ath/ath9k/recv.c
   205          sta = ieee80211_find_sta(sc->hw, hdr->addr2);
   206          if (sta) {
   207                  an = (struct ath_node *) sta->drv_priv;
   208                  if (ds->ds_rxstat.rs_rssi != ATH9K_RSSI_BAD &&

ds->ds_rxstat.rs_rssi is a signed 8 bit so it will never be == ATH9K_RSSI_BAD.

   209                     !ds->ds_rxstat.rs_moreaggr)
   210                          ATH_RSSI_LPF(an->last_rssi, ds->ds_rxstat.rs_rssi);
   211                  last_rssi = an->last_rssi;
   212          }

I would normally just change the declaration to unsigned but it looks like
someone may have chosen to have it signed on purpose.

Another (uglier) option, is to declare ATH9K_RSSI_BAD like this:
#define ATH9K_RSSI_BAD                  ((int8_t)0x80) 

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ath9k-devel] signed vs unsigned bug in ath9k
  2009-12-07 11:07 [ath9k-devel] signed vs unsigned bug in ath9k Dan Carpenter
@ 2009-12-07 15:55 ` Luis R. Rodriguez
  2009-12-08  8:20     ` Dan Carpenter
  0 siblings, 1 reply; 8+ messages in thread
From: Luis R. Rodriguez @ 2009-12-07 15:55 UTC (permalink / raw)
  To: ath9k-devel

On Mon, Dec 07, 2009 at 03:07:36AM -0800, Dan Carpenter wrote:
> drivers/net/wireless/ath/ath9k/recv.c
>    205          sta = ieee80211_find_sta(sc->hw, hdr->addr2);
>    206          if (sta) {
>    207                  an = (struct ath_node *) sta->drv_priv;
>    208                  if (ds->ds_rxstat.rs_rssi != ATH9K_RSSI_BAD &&
> 
> ds->ds_rxstat.rs_rssi is a signed 8 bit so it will never be == ATH9K_RSSI_BAD.

ATH9K_RSSI_BAD is -128 and the minimum value for int8_t is -128 so why is it a bug?

>    209                     !ds->ds_rxstat.rs_moreaggr)
>    210                          ATH_RSSI_LPF(an->last_rssi, ds->ds_rxstat.rs_rssi);
>    211                  last_rssi = an->last_rssi;
>    212          }
> 
> I would normally just change the declaration to unsigned but it looks like
> someone may have chosen to have it signed on purpose.

Yeah it comes from hardware.

  Luis

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ath9k-devel] signed vs unsigned bug in ath9k
  2009-12-07 15:55 ` Luis R. Rodriguez
@ 2009-12-08  8:20     ` Dan Carpenter
  0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2009-12-08  8:20 UTC (permalink / raw)
  To: ath9k-devel

On Mon, Dec 07, 2009 at 07:55:42AM -0800, Luis R. Rodriguez wrote:
> On Mon, Dec 07, 2009 at 03:07:36AM -0800, Dan Carpenter wrote:
> > drivers/net/wireless/ath/ath9k/recv.c
> >    205          sta = ieee80211_find_sta(sc->hw, hdr->addr2);
> >    206          if (sta) {
> >    207                  an = (struct ath_node *) sta->drv_priv;
> >    208                  if (ds->ds_rxstat.rs_rssi != ATH9K_RSSI_BAD &&
> > 
> > ds->ds_rxstat.rs_rssi is a signed 8 bit so it will never be == ATH9K_RSSI_BAD.
> 
> ATH9K_RSSI_BAD is -128 and the minimum value for int8_t is -128 so why is it a bug?
> 

It could be that someone fixed this already in the net tree?  In mainline it's still
positive 128.

#define ATH9K_RSSI_BAD                  0x80

regards,
dan carpenter

> >    209                     !ds->ds_rxstat.rs_moreaggr)
> >    210                          ATH_RSSI_LPF(an->last_rssi, ds->ds_rxstat.rs_rssi);
> >    211                  last_rssi = an->last_rssi;
> >    212          }
> > 
> > I would normally just change the declaration to unsigned but it looks like
> > someone may have chosen to have it signed on purpose.
> 
> Yeah it comes from hardware.
> 
>   Luis

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: signed vs unsigned bug in ath9k
@ 2009-12-08  8:20     ` Dan Carpenter
  0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2009-12-08  8:20 UTC (permalink / raw)
  To: Luis R. Rodriguez
  Cc: ath9k-devel@lists.ath9k.org, linux-wireless@vger.kernel.org,
	Luis Rodriguez, Jouni Malinen, Vasanth Thiagarajan,
	Senthilkumar Balasubramanian, Sujith Manoharan

On Mon, Dec 07, 2009 at 07:55:42AM -0800, Luis R. Rodriguez wrote:
> On Mon, Dec 07, 2009 at 03:07:36AM -0800, Dan Carpenter wrote:
> > drivers/net/wireless/ath/ath9k/recv.c
> >    205          sta = ieee80211_find_sta(sc->hw, hdr->addr2);
> >    206          if (sta) {
> >    207                  an = (struct ath_node *) sta->drv_priv;
> >    208                  if (ds->ds_rxstat.rs_rssi != ATH9K_RSSI_BAD &&
> > 
> > ds->ds_rxstat.rs_rssi is a signed 8 bit so it will never be == ATH9K_RSSI_BAD.
> 
> ATH9K_RSSI_BAD is -128 and the minimum value for int8_t is -128 so why is it a bug?
> 

It could be that someone fixed this already in the net tree?  In mainline it's still
positive 128.

#define ATH9K_RSSI_BAD                  0x80

regards,
dan carpenter

> >    209                     !ds->ds_rxstat.rs_moreaggr)
> >    210                          ATH_RSSI_LPF(an->last_rssi, ds->ds_rxstat.rs_rssi);
> >    211                  last_rssi = an->last_rssi;
> >    212          }
> > 
> > I would normally just change the declaration to unsigned but it looks like
> > someone may have chosen to have it signed on purpose.
> 
> Yeah it comes from hardware.
> 
>   Luis

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ath9k-devel] signed vs unsigned bug in ath9k
  2009-12-08  8:20     ` Dan Carpenter
@ 2009-12-08 14:42       ` John W. Linville
  -1 siblings, 0 replies; 8+ messages in thread
From: John W. Linville @ 2009-12-08 14:42 UTC (permalink / raw)
  To: ath9k-devel

On Tue, Dec 08, 2009 at 10:20:32AM +0200, Dan Carpenter wrote:
> On Mon, Dec 07, 2009 at 07:55:42AM -0800, Luis R. Rodriguez wrote:
> > On Mon, Dec 07, 2009 at 03:07:36AM -0800, Dan Carpenter wrote:
> > > drivers/net/wireless/ath/ath9k/recv.c
> > >    205          sta = ieee80211_find_sta(sc->hw, hdr->addr2);
> > >    206          if (sta) {
> > >    207                  an = (struct ath_node *) sta->drv_priv;
> > >    208                  if (ds->ds_rxstat.rs_rssi != ATH9K_RSSI_BAD &&
> > > 
> > > ds->ds_rxstat.rs_rssi is a signed 8 bit so it will never be == ATH9K_RSSI_BAD.
> > 
> > ATH9K_RSSI_BAD is -128 and the minimum value for int8_t is -128 so why is it a bug?
> > 
> 
> It could be that someone fixed this already in the net tree?  In mainline it's still
> positive 128.
> 
> #define ATH9K_RSSI_BAD                  0x80

I queued a patch to fix that for 2.6.33.

Hth!

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville at tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: signed vs unsigned bug in ath9k
@ 2009-12-08 14:42       ` John W. Linville
  0 siblings, 0 replies; 8+ messages in thread
From: John W. Linville @ 2009-12-08 14:42 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Luis R. Rodriguez, ath9k-devel@lists.ath9k.org,
	linux-wireless@vger.kernel.org, Luis Rodriguez, Jouni Malinen,
	Vasanth Thiagarajan, Senthilkumar Balasubramanian,
	Sujith Manoharan

On Tue, Dec 08, 2009 at 10:20:32AM +0200, Dan Carpenter wrote:
> On Mon, Dec 07, 2009 at 07:55:42AM -0800, Luis R. Rodriguez wrote:
> > On Mon, Dec 07, 2009 at 03:07:36AM -0800, Dan Carpenter wrote:
> > > drivers/net/wireless/ath/ath9k/recv.c
> > >    205          sta = ieee80211_find_sta(sc->hw, hdr->addr2);
> > >    206          if (sta) {
> > >    207                  an = (struct ath_node *) sta->drv_priv;
> > >    208                  if (ds->ds_rxstat.rs_rssi != ATH9K_RSSI_BAD &&
> > > 
> > > ds->ds_rxstat.rs_rssi is a signed 8 bit so it will never be == ATH9K_RSSI_BAD.
> > 
> > ATH9K_RSSI_BAD is -128 and the minimum value for int8_t is -128 so why is it a bug?
> > 
> 
> It could be that someone fixed this already in the net tree?  In mainline it's still
> positive 128.
> 
> #define ATH9K_RSSI_BAD                  0x80

I queued a patch to fix that for 2.6.33.

Hth!

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [ath9k-devel] signed vs unsigned bug in ath9k
  2009-12-08 14:42       ` John W. Linville
@ 2009-12-08 16:08         ` Dan Carpenter
  -1 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2009-12-08 16:08 UTC (permalink / raw)
  To: ath9k-devel

On Tue, Dec 08, 2009 at 09:42:10AM -0500, John W. Linville wrote:
> On Tue, Dec 08, 2009 at 10:20:32AM +0200, Dan Carpenter wrote:
> > On Mon, Dec 07, 2009 at 07:55:42AM -0800, Luis R. Rodriguez wrote:
> > > On Mon, Dec 07, 2009 at 03:07:36AM -0800, Dan Carpenter wrote:
> > > > drivers/net/wireless/ath/ath9k/recv.c
> > > >    205          sta = ieee80211_find_sta(sc->hw, hdr->addr2);
> > > >    206          if (sta) {
> > > >    207                  an = (struct ath_node *) sta->drv_priv;
> > > >    208                  if (ds->ds_rxstat.rs_rssi != ATH9K_RSSI_BAD &&
> > > > 
> > > > ds->ds_rxstat.rs_rssi is a signed 8 bit so it will never be == ATH9K_RSSI_BAD.
> > > 
> > > ATH9K_RSSI_BAD is -128 and the minimum value for int8_t is -128 so why is it a bug?
> > > 
> > 
> > It could be that someone fixed this already in the net tree?  In mainline it's still
> > positive 128.
> > 
> > #define ATH9K_RSSI_BAD                  0x80
> 
> I queued a patch to fix that for 2.6.33.
> 

Grand.  Sorry for the noise.  We static checker people are all finding the same
bugs...

I have been trying to buy enough wifi cards to download the latest git, but 
apparently the English gentleman who generates them has been on a week
long drinking binge.  I have been here asking about it on five days out of the 
last seven...  :/

regards,
dan carpenter

> Hth!
> 
> John
> -- 
> John W. Linville		Someday the world will need a hero, and you
> linville at tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: signed vs unsigned bug in ath9k
@ 2009-12-08 16:08         ` Dan Carpenter
  0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2009-12-08 16:08 UTC (permalink / raw)
  To: John W. Linville
  Cc: Luis R. Rodriguez, ath9k-devel@lists.ath9k.org,
	linux-wireless@vger.kernel.org, Luis Rodriguez, Jouni Malinen,
	Vasanth Thiagarajan, Senthilkumar Balasubramanian,
	Sujith Manoharan

On Tue, Dec 08, 2009 at 09:42:10AM -0500, John W. Linville wrote:
> On Tue, Dec 08, 2009 at 10:20:32AM +0200, Dan Carpenter wrote:
> > On Mon, Dec 07, 2009 at 07:55:42AM -0800, Luis R. Rodriguez wrote:
> > > On Mon, Dec 07, 2009 at 03:07:36AM -0800, Dan Carpenter wrote:
> > > > drivers/net/wireless/ath/ath9k/recv.c
> > > >    205          sta = ieee80211_find_sta(sc->hw, hdr->addr2);
> > > >    206          if (sta) {
> > > >    207                  an = (struct ath_node *) sta->drv_priv;
> > > >    208                  if (ds->ds_rxstat.rs_rssi != ATH9K_RSSI_BAD &&
> > > > 
> > > > ds->ds_rxstat.rs_rssi is a signed 8 bit so it will never be == ATH9K_RSSI_BAD.
> > > 
> > > ATH9K_RSSI_BAD is -128 and the minimum value for int8_t is -128 so why is it a bug?
> > > 
> > 
> > It could be that someone fixed this already in the net tree?  In mainline it's still
> > positive 128.
> > 
> > #define ATH9K_RSSI_BAD                  0x80
> 
> I queued a patch to fix that for 2.6.33.
> 

Grand.  Sorry for the noise.  We static checker people are all finding the same
bugs...

I have been trying to buy enough wifi cards to download the latest git, but 
apparently the English gentleman who generates them has been on a week
long drinking binge.  I have been here asking about it on five days out of the 
last seven...  :/

regards,
dan carpenter

> Hth!
> 
> John
> -- 
> John W. Linville		Someday the world will need a hero, and you
> linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2009-12-08 16:08 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-07 11:07 [ath9k-devel] signed vs unsigned bug in ath9k Dan Carpenter
2009-12-07 15:55 ` Luis R. Rodriguez
2009-12-08  8:20   ` Dan Carpenter
2009-12-08  8:20     ` Dan Carpenter
2009-12-08 14:42     ` [ath9k-devel] " John W. Linville
2009-12-08 14:42       ` John W. Linville
2009-12-08 16:08       ` [ath9k-devel] " Dan Carpenter
2009-12-08 16:08         ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.