Skip to contentSkip to conversation

signed vs unsigned bug in ath9k

ath9k-devel Applied Dan Carpenter 3 participants 5 messages Dec 7, 2009 – Dec 8, 2009

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

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

#1 lore raw
Quoted 7 lines · Dan Carpenter

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?

Quoted 7 lines
   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

author
#2 lore raw
Quoted 11 lines · Luis R. Rodriguez
Quoted 7 lines · Dan Carpenter

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

Quoted 11 lines
Quoted 7 lines
   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

Applied
#3 lore raw
Quoted 10 lines · Dan Carpenter
Quoted 9 lines · Luis R. Rodriguez
Quoted 7 lines · Dan Carpenter

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.

Quoted 7 lines
Quoted 2 lines

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

Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
linville at tuxdriver.com			might be all we have.  Be ready.
author
#4 lore raw
Quoted 14 lines · John W. Linville
Quoted 13 lines · Dan Carpenter
Quoted 11 lines · Luis R. Rodriguez
Quoted 7 lines · Dan Carpenter

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?

Quoted 7 lines
Quoted 4 lines

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

Quoted 6 lines

Hth!

John

Signature · 2 lines
John W. Linville		Someday the world will need a hero, and you
[email protected]			might be all we have.  Be ready.