Re: [patch] ipw2x00: shift wrap bugs setting -rt_tsf
Hello, Joe,
On 24 October 2014 02:43, Joe Perches j...@perches.com wrote:
struct ipw_rt_hdr {
struct ieee80211_radiotap_header rt_hdr;
u64 rt_tsf; /* TSF */ /* XXX */
u8 rt_flags;/* radiotap packet flags *
u8 rt_rate; /* rate in 500kb/s */
__le16 rt_channel; /* channel in mhz */
__le16 rt_chbitmask;/* channel bitfield */
s8 rt_dbmsignal;/* signal in dbM, kluged to signed */
s8 rt_dbmnoise;
u8 rt_antenna; /* antenna number */
u8 payload[0]; /* payload... */
} __packed;
Maybe rt_tsf (which is otherwise unused in this code),
should be __le64 so maybe use (u32) ?
Yes, you are right, the field definition should be __le64 as you
suggest. All values in radiotap header are specified in little endian
byte order according to the documentation at www.radiotap.org.
ipw_rt-rt_txf = cpu_to_le64((u32)(frame-parent_tsf[3] 24 |
frame-parent_tsf[2] 16 |
frame-parent_tsf[1] 8 |
frame-parent_tsf[0]));
That looks fine for me. Will you send a patch?
Stanislav.
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[patch] ipw2x00: shift wrap bugs setting -rt_tsf
The -parent_tsf[] array holds u8 values. It type promoted to int for the shift operation so the 24 shift operation can wrap. The cast needs to be done before the shift instead of after. Signed-off-by: Dan Carpenter dan.carpen...@oracle.com --- Static checker work. Untested. diff --git a/drivers/net/wireless/ipw2x00/ipw2200.c b/drivers/net/wireless/ipw2x00/ipw2200.c index edc3443..7e1f2af 100644 --- a/drivers/net/wireless/ipw2x00/ipw2200.c +++ b/drivers/net/wireless/ipw2x00/ipw2200.c @@ -7819,10 +7819,10 @@ static void ipw_handle_data_packet_monitor(struct ipw_priv *priv, /* Zero the flags, we'll add to them as we go */ ipw_rt-rt_flags = 0; - ipw_rt-rt_tsf = (u64)(frame-parent_tsf[3] 24 | - frame-parent_tsf[2] 16 | - frame-parent_tsf[1] 8 | - frame-parent_tsf[0]); + ipw_rt-rt_tsf = (u64)frame-parent_tsf[3] 24 | + frame-parent_tsf[2] 16 | + frame-parent_tsf[1] 8 | + frame-parent_tsf[0]; /* Convert signal to DBM */ ipw_rt-rt_dbmsignal = antsignal; @@ -8028,10 +8028,10 @@ static void ipw_handle_promiscuous_rx(struct ipw_priv *priv, /* Zero the flags, we'll add to them as we go */ ipw_rt-rt_flags = 0; - ipw_rt-rt_tsf = (u64)(frame-parent_tsf[3] 24 | - frame-parent_tsf[2] 16 | - frame-parent_tsf[1] 8 | - frame-parent_tsf[0]); + ipw_rt-rt_tsf = (u64)frame-parent_tsf[3] 24 | + frame-parent_tsf[2] 16 | + frame-parent_tsf[1] 8 | + frame-parent_tsf[0]; /* Convert to DBM */ ipw_rt-rt_dbmsignal = signal; -- To unsubscribe from this list: send the line unsubscribe linux-wireless in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [patch] ipw2x00: shift wrap bugs setting -rt_tsf
On Fri, 2014-10-24 at 11:15 +0300, Dan Carpenter wrote:
The -parent_tsf[] array holds u8 values. It type promoted to int for
the shift operation so the 24 shift operation can wrap. The cast
needs to be done before the shift instead of after.
Signed-off-by: Dan Carpenter dan.carpen...@oracle.com
---
Static checker work. Untested.
diff --git a/drivers/net/wireless/ipw2x00/ipw2200.c
b/drivers/net/wireless/ipw2x00/ipw2200.c
[]
@@ -7819,10 +7819,10 @@ static void ipw_handle_data_packet_monitor(struct
ipw_priv *priv,
/* Zero the flags, we'll add to them as we go */
ipw_rt-rt_flags = 0;
- ipw_rt-rt_tsf = (u64)(frame-parent_tsf[3] 24 |
-frame-parent_tsf[2] 16 |
-frame-parent_tsf[1] 8 |
-frame-parent_tsf[0]);
+ ipw_rt-rt_tsf = (u64)frame-parent_tsf[3] 24 |
+ frame-parent_tsf[2] 16 |
+ frame-parent_tsf[1] 8 |
+ frame-parent_tsf[0];
/* Convert signal to DBM */
ipw_rt-rt_dbmsignal = antsignal;
@@ -8028,10 +8028,10 @@ static void ipw_handle_promiscuous_rx(struct ipw_priv
*priv,
/* Zero the flags, we'll add to them as we go */
ipw_rt-rt_flags = 0;
- ipw_rt-rt_tsf = (u64)(frame-parent_tsf[3] 24 |
-frame-parent_tsf[2] 16 |
-frame-parent_tsf[1] 8 |
-frame-parent_tsf[0]);
+ ipw_rt-rt_tsf = (u64)frame-parent_tsf[3] 24 |
+ frame-parent_tsf[2] 16 |
+ frame-parent_tsf[1] 8 |
+ frame-parent_tsf[0];
/* Convert to DBM */
ipw_rt-rt_dbmsignal = signal;
struct ipw_rt_hdr {
struct ieee80211_radiotap_header rt_hdr;
u64 rt_tsf; /* TSF */ /* XXX */
u8 rt_flags;/* radiotap packet flags *
u8 rt_rate; /* rate in 500kb/s */
__le16 rt_channel; /* channel in mhz */
__le16 rt_chbitmask;/* channel bitfield */
s8 rt_dbmsignal;/* signal in dbM, kluged to signed */
s8 rt_dbmnoise;
u8 rt_antenna; /* antenna number */
u8 payload[0]; /* payload... */
} __packed;
Maybe rt_tsf (which is otherwise unused in this code),
should be __le64 so maybe use (u32) ?
ipw_rt-rt_txf = cpu_to_le64((u32)(frame-parent_tsf[3] 24 |
frame-parent_tsf[2] 16 |
frame-parent_tsf[1] 8 |
frame-parent_tsf[0]));
Al Viro touched this with commit 83f7d57c and added the XXX
when he did a bunch of type conversions from ufoo to __lefoo
Dunno what's right.
--
To unsubscribe from this list: send the line unsubscribe linux-wireless in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
