| Message ID | 1459870106-2694-1-git-send-email-mohammed@qca.qualcomm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Tue, 2016-04-05 at 20:58 +0530, Mohammed Shafi Shajakhan wrote: > From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> > > Return value is incorrect for btcoex and peer stats debugfs > 'write' entries if the user provides a value that matches with > the already available debugfs entry, this results in the debugfs > entry getting stuck and the operation has to be terminated manually. > Fix this by returning the appropriate return 'count' as we do it for > other debugfs entries like pktlog etc Not your code, but some of the xor uses are odd at best. > diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c [] > @@ -2122,7 +2122,7 @@ static ssize_t ath10k_write_btcoex(struct file *file, > struct ath10k *ar = file->private_data; > char buf[32]; > size_t buf_size; > - int ret = 0; > + int ret; > bool val; > > buf_size = min(count, (sizeof(buf) - 1)); > @@ -2142,8 +2142,10 @@ static ssize_t ath10k_write_btcoex(struct file *file, > goto exit; > } > > - if (!(test_bit(ATH10K_FLAG_BTCOEX, &ar->dev_flags) ^ val)) > + if (!(test_bit(ATH10K_FLAG_BTCOEX, &ar->dev_flags) ^ val)) { xor on an int and a bool. > @@ -2189,7 +2191,7 @@ static ssize_t ath10k_write_peer_stats(struct file *file, > struct ath10k *ar = file->private_data; > char buf[32]; > size_t buf_size; > - int ret = 0; > + int ret; > bool val; > > buf_size = min(count, (sizeof(buf) - 1)); > @@ -2209,8 +2211,10 @@ static ssize_t ath10k_write_peer_stats(struct file *file, > goto exit; > } > > - if (!(test_bit(ATH10K_FLAG_PEER_STATS, &ar->dev_flags) ^ val)) > + if (!(test_bit(ATH10K_FLAG_PEER_STATS, &ar->dev_flags) ^ val)) { here too
Hello Joe, On Tue, Apr 05, 2016 at 10:28:56AM -0700, Joe Perches wrote: > On Tue, 2016-04-05 at 20:58 +0530, Mohammed Shafi Shajakhan wrote: > > From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> > > > > Return value is incorrect for btcoex and peer stats debugfs > > 'write' entries if the user provides a value that matches with > > the already available debugfs entry, this results in the debugfs > > entry getting stuck and the operation has to be terminated manually. > > Fix this by returning the appropriate return 'count' as we do it for > > other debugfs entries like pktlog etc > > Not your code, but some of the xor uses are odd at best. [shafi] ok. > > > diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c > [] > > @@ -2122,7 +2122,7 @@ static ssize_t ath10k_write_btcoex(struct file *file, > > struct ath10k *ar = file->private_data; > > char buf[32]; > > size_t buf_size; > > - int ret = 0; > > + int ret; > > bool val; > > > > buf_size = min(count, (sizeof(buf) - 1)); > > @@ -2142,8 +2142,10 @@ static ssize_t ath10k_write_btcoex(struct file *file, > > goto exit; > > } > > > > - if (!(test_bit(ATH10K_FLAG_BTCOEX, &ar->dev_flags) ^ val)) > > + if (!(test_bit(ATH10K_FLAG_BTCOEX, &ar->dev_flags) ^ val)) { > > xor on an int and a bool. [shafi] Should we change the 'val' to int, but this code seems to be working test_bit seems to do this : return 1UL & (addr[BIT_WORD(nr)] >> (nr & (BITS_PER_LONG-1))); do we see a problem, it should work right (because they are bitwise 'ANDING' with 1UL) > > > @@ -2189,7 +2191,7 @@ static ssize_t ath10k_write_peer_stats(struct file *file, > > struct ath10k *ar = file->private_data; > > char buf[32]; > > size_t buf_size; > > - int ret = 0; > > + int ret; > > bool val; > > > > buf_size = min(count, (sizeof(buf) - 1)); > > @@ -2209,8 +2211,10 @@ static ssize_t ath10k_write_peer_stats(struct file *file, > > goto exit; > > } > > > > - if (!(test_bit(ATH10K_FLAG_PEER_STATS, &ar->dev_flags) ^ val)) > > + if (!(test_bit(ATH10K_FLAG_PEER_STATS, &ar->dev_flags) ^ val)) { > > here too [shafi] same thing here ? is there a better code , kindly suggest > > -- > 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
Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> writes: > From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> > > Return value is incorrect for btcoex and peer stats debugfs > 'write' entries if the user provides a value that matches with > the already available debugfs entry, this results in the debugfs > entry getting stuck and the operation has to be terminated manually. > Fix this by returning the appropriate return 'count' as we do it for > other debugfs entries like pktlog etc > > Fixes: cc61a1bbbc0e ("ath10k: enable debugfs provision to enable > Peer Stats feature") > Fixes: c28e6f06ff40 ("ath10k: fix sanity check on enabling btcoex via > debugfs") > Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com> Applied, thanks. I removed the line wrapping from "Fixes:" lines.
Applied, thanks. I removed the line wrapping from "Fixes:" lines. [shafi] thank you Kalle.
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c index 76bbe17..e7d441c 100644 --- a/drivers/net/wireless/ath/ath10k/debug.c +++ b/drivers/net/wireless/ath/ath10k/debug.c @@ -2122,7 +2122,7 @@ static ssize_t ath10k_write_btcoex(struct file *file, struct ath10k *ar = file->private_data; char buf[32]; size_t buf_size; - int ret = 0; + int ret; bool val; buf_size = min(count, (sizeof(buf) - 1)); @@ -2142,8 +2142,10 @@ static ssize_t ath10k_write_btcoex(struct file *file, goto exit; } - if (!(test_bit(ATH10K_FLAG_BTCOEX, &ar->dev_flags) ^ val)) + if (!(test_bit(ATH10K_FLAG_BTCOEX, &ar->dev_flags) ^ val)) { + ret = count; goto exit; + } if (val) set_bit(ATH10K_FLAG_BTCOEX, &ar->dev_flags); @@ -2189,7 +2191,7 @@ static ssize_t ath10k_write_peer_stats(struct file *file, struct ath10k *ar = file->private_data; char buf[32]; size_t buf_size; - int ret = 0; + int ret; bool val; buf_size = min(count, (sizeof(buf) - 1)); @@ -2209,8 +2211,10 @@ static ssize_t ath10k_write_peer_stats(struct file *file, goto exit; } - if (!(test_bit(ATH10K_FLAG_PEER_STATS, &ar->dev_flags) ^ val)) + if (!(test_bit(ATH10K_FLAG_PEER_STATS, &ar->dev_flags) ^ val)) { + ret = count; goto exit; + } if (val) set_bit(ATH10K_FLAG_PEER_STATS, &ar->dev_flags);