randconfig build warnings in staging/iio/meter/
Hello, With next-20141215, i have got the following build warnings: drivers/staging/iio/meter/ade7754.c:222:6: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized] drivers/staging/iio/meter/ade7754.c:368:6: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized] drivers/staging/iio/meter/ade7759.c:224:6: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized] drivers/staging/iio/meter/ade7759.c:309:6: warning: ‘val’ may be used uninitialized in this function [-Wmaybe-uninitialized] I have attached the randconfig file. The possible fix for ade7754.c is to check the return value of ade7754_spi_read_reg_8 function at line 222, and at line 368. The possible fix for ade7759.c is to check the return value of ade7759_spi_read_reg_16 function at line 224, and at line 309. Thanks, rand_config1 Description: Binary data ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: randconfig build warnings in staging/iio/meter/
On Mon, Dec 15, 2014 at 10:25 AM, devendra.aaru wrote: > Hello, > > With next-20141215, i have got the following build warnings: > > > drivers/staging/iio/meter/ade7754.c:222:6: warning: ‘val’ may be used > uninitialized in this function [-Wmaybe-uninitialized] > drivers/staging/iio/meter/ade7754.c:368:6: warning: ‘val’ may be used > uninitialized in this function [-Wmaybe-uninitialized] > drivers/staging/iio/meter/ade7759.c:224:6: warning: ‘val’ may be used > uninitialized in this function [-Wmaybe-uninitialized] > drivers/staging/iio/meter/ade7759.c:309:6: warning: ‘val’ may be used > uninitialized in this function [-Wmaybe-uninitialized] > > I have attached the randconfig file. > > The possible fix for ade7754.c is to check the return value of > ade7754_spi_read_reg_8 function at line 222, and at line 368. > > The possible fix for ade7759.c is to check the return value of > ade7759_spi_read_reg_16 function at line 224, and at line 309. Hi, Thanks for reporting! We are working on IIO cleanup project [1] and we can take care of fixing this. thanks, Daniel. [1] http://iiobits.wordpress.com/ ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: randconfig build warnings in staging/iio/meter/
> Hi, > > Thanks for reporting! We are working on IIO cleanup project [1] > and we can take care of fixing this. > Thanks for a very quick response. I did fix those warnings. Let me know if i need to send patches to iio ml. --Devendra ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: randconfig build warnings in staging/iio/meter/
On Mon, Dec 15, 2014 at 10:48 AM, devendra.aaru wrote: >> Hi, >> >> Thanks for reporting! We are working on IIO cleanup project [1] >> and we can take care of fixing this. >> > > Thanks for a very quick response. I did fix those warnings. Let me > know if i need to send patches to iio ml. All right. Send them to Greg (CCed) + linux-iio for review. thanks, Daniel. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: lustre: osc: lproc_osc.c: Fix for possible null pointer dereference
On Sun, Dec 14, 2014 at 11:37:18PM +0100, Rickard Strandqvist wrote:
> There is otherwise a risk of a possible null pointer dereference.
>
> Was largely found by using a static code analysis program called cppcheck.
>
> Signed-off-by: Rickard Strandqvist
> ---
> drivers/staging/lustre/lustre/osc/lproc_osc.c |4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/staging/lustre/lustre/osc/lproc_osc.c
> b/drivers/staging/lustre/lustre/osc/lproc_osc.c
> index 9f719bc..9ba6293 100644
> --- a/drivers/staging/lustre/lustre/osc/lproc_osc.c
> +++ b/drivers/staging/lustre/lustre/osc/lproc_osc.c
> @@ -237,13 +237,15 @@ static ssize_t osc_cur_grant_bytes_seq_write(struct
> file *file, const char *buff
> size_t count, loff_t *off)
> {
> struct obd_device *obd = ((struct seq_file
> *)file->private_data)->private;
> - struct client_obd *cli = &obd->u.cli;
> + struct client_obd *cli;
This isn't really a dereference. You're just getting the address of
obd->u.cli. So if obd is NULL then it won't crash.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference
On Sun, Dec 14, 2014 at 11:39:14PM +0100, Rickard Strandqvist wrote: > There is otherwise a risk of a possible null pointer dereference. > > Was largely found by using a static code analysis program called cppcheck. > > Signed-off-by: Rickard Strandqvist > --- > drivers/staging/rtl8723au/os_dep/usb_intf.c |3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8723au/os_dep/usb_intf.c > b/drivers/staging/rtl8723au/os_dep/usb_intf.c > index 865743e..71a6330 100644 > --- a/drivers/staging/rtl8723au/os_dep/usb_intf.c > +++ b/drivers/staging/rtl8723au/os_dep/usb_intf.c > @@ -351,10 +351,11 @@ error_exit: > int rtw_hw_resume23a(struct rtw_adapter *padapter) That's weird. Is this function even called? regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch giving bogus advice (was staging: goldfish: Fix minor coding style)
On Sat, 13 Dec 2014 11:46:47 -0800 Jeremiah Mahler wrote: > Loïc, > > On Sat, Dec 13, 2014 at 07:22:38PM +0100, Loic Pefferkorn wrote: > > > Whose convention is this? I can't find any mention in > > > Documention/CodingStyle. checkpatch.pl doesn't complain about them. > > > And there are almost three thousand examples in staging which don't > > > use this convention. > > > > > > linux-next$ grep -r "== NULL" drivers/staging/* | wc -l > > > 2844 > > > > Hi Jeremiah, > > > > Thanks for your feedback. > > > > I have used checkpatch.pl with the --strict flag: checkpatch.pl is a bit dubious at the best of times - you can't automate taste without an AI ;). With --strict it's a positive hazard. Those kind of small cleanups really only make sense if you are doing big changes to the code itself anyway and are doing testing and all the rest. In this case I'd say checkpatch.pl is actually wrong because in the general case it's better to compare with NULL in C If you write if (!x) and accidentally use a non-pointer type you don't get a warning. If you try and compare a non pointer type to NULL you usually do. So the NULL comparison avoids accidents. The historical reason for it being done in C was I think to avoid the if (x = NULL) bug, but gcc will shout at you for that these days. Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: goldfish: Fix minor coding style
On Sat, 13 Dec 2014 22:20:52 +0100 Loic Pefferkorn wrote: > On Sat, Dec 13, 2014 at 07:07:05PM +, One Thousand Gnomes wrote: > > > > Pointless churn. It makes it less readable if anything, and it removes > > the type safety as you are now checking against 0 not (void *)0 > > > > NAK > > > > Alan > > The type safety is an interesting point I was not aware of. > > Just in case, do you have something for me on your todo list? Depends what you feel confident tackling. An interesting but challenging one to look at as a newcomer might be https://bugzilla.kernel.org/show_bug.cgi?id=87951 because it provides you with an example .iso file you can loopback mount to see the crash, and the same fixes were done for a similar bug so can be seen in the git log to work from. commit 410dd3cf4c9b36f27ed4542ee18b1af5e68645a4 It's predictable, it's repeatable and it can be done crashing a virtual machine not a real one. That usually makes things eaiser to fix. For something easier perhaps https://bugzilla.kernel.org/show_bug.cgi?id=75111 which just needs the configuration help fixing Alan ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch giving bogus advice (was staging: goldfish: Fix minor coding style)
I haven't seen any bugs caused by lack of type safety with "!foo"... I prefer !foo because it is more common in the kernel and I think it's easier to read but I don't feel strongly about this. I kind of hate "if (foo != NULL) though, because it's a double negative. But I really hate when people start adding the "!= 0" on to all their conditions. if (frob() != 0) Also: if (a + b != 0) People do this all the time instead of "if (a || b)" and I don't know why... regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: goldfish: Fix minor coding style
On Mon, Dec 15, 2014 at 11:51:47AM +, One Thousand Gnomes wrote: > > Depends what you feel confident tackling. An interesting but > challenging one to look at as a newcomer might be > > https://bugzilla.kernel.org/show_bug.cgi?id=87951 > > because it provides you with an example .iso file you can loopback mount > to see the crash, and the same fixes were done for a similar bug so can > be seen in the git log to work from. > > commit 410dd3cf4c9b36f27ed4542ee18b1af5e68645a4 > > It's predictable, it's repeatable and it can be done crashing a virtual > machine not a real one. That usually makes things eaiser to fix. > > For something easier perhaps > > https://bugzilla.kernel.org/show_bug.cgi?id=75111 > > which just needs the configuration help fixing > > Alan > Hi Alan, Your answer is greatly appreciated, I'm going to do my best! -- Cheers, Loïc ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch giving bogus advice (was staging: goldfish: Fix minor coding style)
On Mon, Dec 15, 2014 at 11:44:21AM +, One Thousand Gnomes wrote: > On Sat, 13 Dec 2014 11:46:47 -0800 > Jeremiah Mahler wrote: > > > Loïc, > > > > On Sat, Dec 13, 2014 at 07:22:38PM +0100, Loic Pefferkorn wrote: > > > > Whose convention is this? I can't find any mention in > > > > Documention/CodingStyle. checkpatch.pl doesn't complain about them. > > > > And there are almost three thousand examples in staging which don't > > > > use this convention. > > > > > > > > linux-next$ grep -r "== NULL" drivers/staging/* | wc -l > > > > 2844 > > > > > > Hi Jeremiah, > > > > > > Thanks for your feedback. > > > > > > I have used checkpatch.pl with the --strict flag: > > checkpatch.pl is a bit dubious at the best of times - you can't automate > taste without an AI ;). With --strict it's a positive hazard. > > Those kind of small cleanups really only make sense if you are doing big > changes to the code itself anyway and are doing testing and all the rest. > > In this case I'd say checkpatch.pl is actually wrong because in the > general case it's better to compare with NULL in C > > If you write > > if (!x) > > and accidentally use a non-pointer type you don't get a warning. If you > try and compare a non pointer type to NULL you usually do. So the NULL > comparison avoids accidents. > > The historical reason for it being done in C was I think to avoid the > > if (x = NULL) > > bug, but gcc will shout at you for that these days. > Or another way mentioned in K&R that produces a compile error if (NULL = x) > Alan > > > -- - Jeremiah Mahler ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch giving bogus advice (was staging: goldfish: Fix minor coding style)
On Mon, Dec 15, 2014 at 05:03:46AM -0800, Jeremiah Mahler wrote:
>
> Or another way mentioned in K&R that produces a compile error
>
> if (NULL = x)
>
Yes. People used to write Yoda code back in the day. Don't ever do
this in the kernel.
1) It looks stupid.
2) GCC will catch most == vs = bugs as Alan pointed out.
3) There are still some that sneak through because people put double
parenthesis around everything like "if ((foo = NULL) || (...)) {",
but checkpatch.pl and Smatch will catch those.
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/2] staging: lustre: Fix minor style error in libcfs_string.c
Signed-off-by: Matthew Tyler --- drivers/staging/lustre/lustre/libcfs/libcfs_string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c index fb88733..e67a18d 100644 --- a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c +++ b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c @@ -132,7 +132,7 @@ char *cfs_firststr(char *str, size_t size) ++end; } - *end= '\0'; + *end = '\0'; out: return str; } -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch giving bogus advice (was staging: goldfish: Fix minor coding style)
Dan,
On Mon, Dec 15, 2014 at 04:43:34PM +0300, Dan Carpenter wrote:
> On Mon, Dec 15, 2014 at 05:03:46AM -0800, Jeremiah Mahler wrote:
> >
> > Or another way mentioned in K&R that produces a compile error
> >
> > if (NULL = x)
> >
>
> Yes. People used to write Yoda code back in the day. Don't ever do
> this in the kernel.
>
> 1) It looks stupid.
Agreed :-)
> 2) GCC will catch most == vs = bugs as Alan pointed out.
> 3) There are still some that sneak through because people put double
>parenthesis around everything like "if ((foo = NULL) || (...)) {",
>but checkpatch.pl and Smatch will catch those.
>
> regards,
> dan carpenter
>
--
- Jeremiah Mahler
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/2] staging: lustre: Cleanup cfs_str2mask in libcfs_string.c
- Replace body-less for-loop with while loop
- Use '\0' for null character instead of 0
Signed-off-by: Matthew Tyler
---
drivers/staging/lustre/lustre/libcfs/libcfs_string.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
index e67a18d..b916c2a 100644
--- a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
+++ b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
@@ -47,7 +47,7 @@ int cfs_str2mask(const char *str, const char *(*bit2str)(int
bit),
int *oldmask, int minmask, int allmask)
{
const char *debugstr;
- char op = 0;
+ char op = '\0';
int newmask = minmask, i, len, found = 0;
/* must be a list of tokens separated by whitespace
@@ -55,10 +55,10 @@ int cfs_str2mask(const char *str, const char
*(*bit2str)(int bit),
* appears first in , '*oldmask' is used as the starting point
* (relative), otherwise minmask is used (absolute). An operator
* applies to all following tokens up to the next operator. */
- while (*str != 0) {
+ while (*str != '\0') {
while (isspace(*str))
str++;
- if (*str == 0)
+ if (*str == '\0')
break;
if (*str == '+' || *str == '-') {
op = *str++;
@@ -67,13 +67,15 @@ int cfs_str2mask(const char *str, const char
*(*bit2str)(int bit),
newmask = *oldmask;
while (isspace(*str))
str++;
- if (*str == 0)/* trailing op */
+ if (*str == '\0') /* trailing op */
return -EINVAL;
}
/* find token length */
- for (len = 0; str[len] != 0 && !isspace(str[len]) &&
- str[len] != '+' && str[len] != '-'; len++);
+ len = 0;
+ while (str[len] != '\0' && !isspace(str[len]) &&
+ str[len] != '+' && str[len] != '-')
+ len++;
/* match token */
found = 0;
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: lustre: Cleanup cfs_str2mask in libcfs_string.c
On Mon, Dec 15, 2014 at 10:05:34PM +0800, Matthew Tyler wrote: > /* find token length */ > - for (len = 0; str[len] != 0 && !isspace(str[len]) && > - str[len] != '+' && str[len] != '-'; len++); > + len = 0; > + while (str[len] != '\0' && !isspace(str[len]) && > + str[len] != '+' && str[len] != '-') This isn't aligned correctly. I think checkpatch.pl --strict will warn here. It should be: while (str[len] != '\0' && !isspace(str[len]) && str[len] != '+' && str[len] != '-') [tab][tab][space][space][space][space][space][space][space]str[len] ... Otherwise, it looks good. regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 2/2] staging: lustre: Cleanup cfs_str2mask in libcfs_string.c
Cheers. I was checking with --terse and that warning was not showing. I'll just fix that up now. Thanks for your help! New to this... Matthew Tyler Mob: +61407964325 Site: http://matthewtyler.io/ On Mon, Dec 15, 2014 at 10:13 PM, Dan Carpenter wrote: > On Mon, Dec 15, 2014 at 10:05:34PM +0800, Matthew Tyler wrote: >> /* find token length */ >> - for (len = 0; str[len] != 0 && !isspace(str[len]) && >> - str[len] != '+' && str[len] != '-'; len++); >> + len = 0; >> + while (str[len] != '\0' && !isspace(str[len]) && >> + str[len] != '+' && str[len] != '-') > > This isn't aligned correctly. I think checkpatch.pl --strict will warn > here. It should be: > > while (str[len] != '\0' && !isspace(str[len]) && >str[len] != '+' && str[len] != '-') > > [tab][tab][space][space][space][space][space][space][space]str[len] ... > > Otherwise, it looks good. > > regards, > dan carpenter > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 1/2] staging: lustre: Fix minor style error in libcfs_string.c
Signed-off-by: Matthew Tyler --- drivers/staging/lustre/lustre/libcfs/libcfs_string.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c index fb88733..e67a18d 100644 --- a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c +++ b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c @@ -132,7 +132,7 @@ char *cfs_firststr(char *str, size_t size) ++end; } - *end= '\0'; + *end = '\0'; out: return str; } -- 2.1.3 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v3 2/2] staging: lustre: Cleanup cfs_str2mask in libcfs_string.c
- Replace body-less for-loop with while loop
- Use '\0' for null character instead of 0
Signed-off-by: Matthew Tyler
---
drivers/staging/lustre/lustre/libcfs/libcfs_string.c | 14 --
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
index e67a18d..76d4392 100644
--- a/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
+++ b/drivers/staging/lustre/lustre/libcfs/libcfs_string.c
@@ -47,7 +47,7 @@ int cfs_str2mask(const char *str, const char *(*bit2str)(int
bit),
int *oldmask, int minmask, int allmask)
{
const char *debugstr;
- char op = 0;
+ char op = '\0';
int newmask = minmask, i, len, found = 0;
/* must be a list of tokens separated by whitespace
@@ -55,10 +55,10 @@ int cfs_str2mask(const char *str, const char
*(*bit2str)(int bit),
* appears first in , '*oldmask' is used as the starting point
* (relative), otherwise minmask is used (absolute). An operator
* applies to all following tokens up to the next operator. */
- while (*str != 0) {
+ while (*str != '\0') {
while (isspace(*str))
str++;
- if (*str == 0)
+ if (*str == '\0')
break;
if (*str == '+' || *str == '-') {
op = *str++;
@@ -67,13 +67,15 @@ int cfs_str2mask(const char *str, const char
*(*bit2str)(int bit),
newmask = *oldmask;
while (isspace(*str))
str++;
- if (*str == 0)/* trailing op */
+ if (*str == '\0') /* trailing op */
return -EINVAL;
}
/* find token length */
- for (len = 0; str[len] != 0 && !isspace(str[len]) &&
- str[len] != '+' && str[len] != '-'; len++);
+ len = 0;
+ while (str[len] != '\0' && !isspace(str[len]) &&
+ str[len] != '+' && str[len] != '-')
+ len++;
/* match token */
found = 0;
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 2/2] staging: lustre: Cleanup cfs_str2mask in libcfs_string.c
Looks good. Thanks! regards, dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8723au: Fix sparse warnings
Some struct fields in wifi.h are meant to be __le16 but were declared as
unsigned short. This was reported by sparse:
rtw_wlan_util.c:538:24: warning: cast to restricted __le16
rtw_wlan_util.c:1544:29: warning: cast to restricted __le16
rtw_wlan_util.c:1546:25: warning: cast to restricted __le16
This patch updates the types of the fields in `AC_param` and
`ADDBA_request` structs to be consistent with relevant structs in
include/linux/ieee80211.h.
Signed-off-by: Krzysztof Konopko
---
drivers/staging/rtl8723au/include/wifi.h | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/rtl8723au/include/wifi.h
b/drivers/staging/rtl8723au/include/wifi.h
index fd3da3b..25d573c 100644
--- a/drivers/staging/rtl8723au/include/wifi.h
+++ b/drivers/staging/rtl8723au/include/wifi.h
@@ -26,9 +26,9 @@
--*/
struct AC_param {
- unsigned char ACI_AIFSN;
- unsigned char CW;
- unsigned short TXOP_limit;
+ u8 ACI_AIFSN;
+ u8 CW;
+ __le16 TXOP_limit;
} __packed;
struct WMM_para_element {
@@ -38,10 +38,10 @@ struct WMM_para_element {
} __packed;
struct ADDBA_request {
- unsigned char dialog_token;
- unsigned short BA_para_set;
- unsigned short BA_timeout_value;
- unsigned short BA_starting_seqctrl;
+ u8 dialog_token;
+ __le16 BA_para_set;
+ __le16 BA_timeout_value;
+ __le16 BA_starting_seqctrl;
} __packed;
--
2.1.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fix sparse warnings
Krzysztof Konopko writes: > On 12/12/14 19:52, Jes Sorensen wrote: >> Larry Finger writes: >>> On 12/12/2014 05:35 AM, Krzysztof Konopko wrote: I was hunting particularly for inconsistencies with `sparse` and came across this one. But I dug a bit further and I wonder why the driver is not using standard stuff like the one in `include/linux/ieee80211.h` where any data wider than one byte is clearly declared as __le? >>> >>> That is a good question. One possibility is that those definitions do >>> not exist on some of the older kernels that Realtek supports. They >>> generally work with 2.6.18 and newer. >> >> The reason the 8723au driver doesn't use the defines from there is that >> in ieee80211.h they are part of struct ieee80211_mgmt, while the 8723au >> driver access the addba etc. elements without the full struct in place. >> > > And why is that the case? > (I'm trying to understand, not debunk) > > Looks to me that this driver has been kept out of the tree for quite a > while (by Realtek) and now suffers from locally invented stuff. I > understand this is a lot of work to unify the codebase with ieee80211.h, > but are there any technical hurdles? I'm just curious. The main issue is that the RTL driver maintains a partial copy of the management frame, ie. without the front block containing the MAC addresses. Switching this over to carry a full copy of the frame is extremely intrusive as it's mixed in pretty much everywhere in the driver. The driver is derived from Realtek's multi-OS vendor driver, which included code for pretty much every OS on the planet. If you want to see how it looked initially, check out Larry's github tree and go back to the initial checkins Realtek seem to copy it into a new tree and devel from there whenever they do a new chip, so the legacy in this codebase is huge. Cheers, Jes ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fix sparse warnings
Krzysztof Konopko writes:
> Some struct fields in wifi.h are meant to be __le16 but were declared as
> unsigned short. This was reported by sparse:
>
> rtw_wlan_util.c:538:24: warning: cast to restricted __le16
> rtw_wlan_util.c:1544:29: warning: cast to restricted __le16
> rtw_wlan_util.c:1546:25: warning: cast to restricted __le16
>
> This patch changes the types of the struct fields involved to be
> little-endian which is what is received over the air and consistent with
> relevant structs in include/linux/ieee80211.h.
>
> Signed-off-by: Krzysztof Konopko
> ---
> drivers/staging/rtl8723au/include/wifi.h | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
Looks OK
Signed-off-by: Jes Sorensen
>
> diff --git a/drivers/staging/rtl8723au/include/wifi.h
> b/drivers/staging/rtl8723au/include/wifi.h
> index fd3da3b..266c43e 100644
> --- a/drivers/staging/rtl8723au/include/wifi.h
> +++ b/drivers/staging/rtl8723au/include/wifi.h
> @@ -28,7 +28,7 @@
> struct AC_param {
> unsigned char ACI_AIFSN;
> unsigned char CW;
> - unsigned short TXOP_limit;
> + __le16 TXOP_limit;
> } __packed;
>
> struct WMM_para_element {
> @@ -39,9 +39,9 @@ struct WMM_para_element {
>
> struct ADDBA_request {
> unsigned char dialog_token;
> - unsigned short BA_para_set;
> - unsigned short BA_timeout_value;
> - unsigned short BA_starting_seqctrl;
> + __le16 BA_para_set;
> + __le16 BA_timeout_value;
> + __le16 BA_starting_seqctrl;
> } __packed;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: Fix sparse warnings
Krzysztof Konopko writes:
> Some struct fields in wifi.h are meant to be __le16 but were declared as
> unsigned short. This was reported by sparse:
>
> rtw_wlan_util.c:538:24: warning: cast to restricted __le16
> rtw_wlan_util.c:1544:29: warning: cast to restricted __le16
> rtw_wlan_util.c:1546:25: warning: cast to restricted __le16
>
> This patch updates the types of the fields in `AC_param` and
> `ADDBA_request` structs to be consistent with relevant structs in
> include/linux/ieee80211.h.
>
> Signed-off-by: Krzysztof Konopko
> ---
> drivers/staging/rtl8723au/include/wifi.h | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
Well the u8 change is really in the nit picking space, but I am fine
with that too.
Signed-off-by: Jes Sorensen
>
> diff --git a/drivers/staging/rtl8723au/include/wifi.h
> b/drivers/staging/rtl8723au/include/wifi.h
> index fd3da3b..25d573c 100644
> --- a/drivers/staging/rtl8723au/include/wifi.h
> +++ b/drivers/staging/rtl8723au/include/wifi.h
> @@ -26,9 +26,9 @@
>
> --*/
>
> struct AC_param {
> - unsigned char ACI_AIFSN;
> - unsigned char CW;
> - unsigned short TXOP_limit;
> + u8 ACI_AIFSN;
> + u8 CW;
> + __le16 TXOP_limit;
> } __packed;
>
> struct WMM_para_element {
> @@ -38,10 +38,10 @@ struct WMM_para_element {
> } __packed;
>
> struct ADDBA_request {
> - unsigned char dialog_token;
> - unsigned short BA_para_set;
> - unsigned short BA_timeout_value;
> - unsigned short BA_starting_seqctrl;
> + u8 dialog_token;
> + __le16 BA_para_set;
> + __le16 BA_timeout_value;
> + __le16 BA_starting_seqctrl;
> } __packed;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference
Dan Carpenter writes:
> On Sun, Dec 14, 2014 at 11:39:14PM +0100, Rickard Strandqvist wrote:
>> There is otherwise a risk of a possible null pointer dereference.
>>
>> Was largely found by using a static code analysis program called cppcheck.
>>
>> Signed-off-by: Rickard Strandqvist
>> ---
>> drivers/staging/rtl8723au/os_dep/usb_intf.c |3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/rtl8723au/os_dep/usb_intf.c
>> b/drivers/staging/rtl8723au/os_dep/usb_intf.c
>> index 865743e..71a6330 100644
>> --- a/drivers/staging/rtl8723au/os_dep/usb_intf.c
>> +++ b/drivers/staging/rtl8723au/os_dep/usb_intf.c
>> @@ -351,10 +351,11 @@ error_exit:
>> int rtw_hw_resume23a(struct rtw_adapter *padapter)
>
> That's weird. Is this function even called?
[jes@ultrasam jes.git]$ find drivers/staging/rtl8723au -name \*.[ch] |xargs
grep rtw_hw_resume
drivers/staging/rtl8723au/include/osdep_intf.h:int rtw_hw_resume23a(struct
rtw_adapter *padapter);
drivers/staging/rtl8723au/os_dep/usb_intf.c:int rtw_hw_resume23a(struct
rtw_adapter *padapter)
drivers/staging/rtl8723au/os_dep/usb_intf.c:DBG_8723A("==>
rtw_hw_resume23a\n");
[jes@ultrasam jes.git]$ find drivers/staging/rtl8723au -name \*.[ch] |xargs
grep rtw_hw_suspend
drivers/staging/rtl8723au/include/osdep_intf.h:int rtw_hw_suspend23a(struct
rtw_adapter *padapter);
drivers/staging/rtl8723au/os_dep/usb_intf.c:int rtw_hw_suspend23a(struct
rtw_adapter *padapter)
drivers/staging/rtl8723au/os_dep/usb_intf.c:DBG_8723A("==>
rtw_hw_suspend23a\n");
A more useful patch would be one removing those two functions IMHO.
Jes
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V3 1/1] Drivers: hv: vmbus: Fix a bug in vmbus_establish_gpadl()
On Sun, Dec 14, 2014 at 11:59:19PM -0800, Jeremiah Mahler wrote: > KY Srinivasan, > > On Mon, Dec 15, 2014 at 07:00:45AM +, KY Srinivasan wrote: > > > > > > > -Original Message- > > > From: Jeremiah Mahler [mailto:jmmah...@gmail.com] > > > Sent: Wednesday, December 10, 2014 6:10 PM > > > To: KY Srinivasan > > > Cc: gre...@linuxfoundation.org; linux-ker...@vger.kernel.org; > > > de...@linuxdriverproject.org; o...@aepfle.de; a...@canonical.com; > > > jasow...@redhat.com; mc...@ipxe.org > > > Subject: Re: [PATCH V3 1/1] Drivers: hv: vmbus: Fix a bug in > > > vmbus_establish_gpadl() > > > > > > K. Y. Srinivasan, > > > > > > On Wed, Dec 10, 2014 at 05:13:00PM -0800, K. Y. Srinivasan wrote: > > > > Correctly compute the local (gpadl) handle. > > > > > > This description is still too sparse for me. How was it computed before > > > and > > > why was this incorrect? Pretend like you are trying to explain your > > > patch to > > > someone who has no idea what you did. > > > > > > > I would like to thank Michael Brown for seeing this > > > > bug. > > > > > > > > Signed-off-by: K. Y. Srinivasan > > > > Reported-by: Michael Brown > > > > --- > > > > Changes in V2: Added the Reported-by tag. > > > > Changes in V3: Cleaned up the commit log. > > > > > > > > drivers/hv/channel.c |4 ++-- > > > > 1 files changed, 2 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index > > > > 433f72a..c76ffbe 100644 > > > > --- a/drivers/hv/channel.c > > > > +++ b/drivers/hv/channel.c > > > > @@ -366,8 +366,8 @@ int vmbus_establish_gpadl(struct vmbus_channel > > > *channel, void *kbuffer, > > > > unsigned long flags; > > > > int ret = 0; > > > > > > > > - next_gpadl_handle = > > > atomic_read(&vmbus_connection.next_gpadl_handle); > > > > - atomic_inc(&vmbus_connection.next_gpadl_handle); > > > > + next_gpadl_handle = > > > > + > > > (atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1); > > > > > > > Tell me if I understand this correctly. > > > > > > Before it read the handle and incremented it. > > > > > > y = x + 1 > > > > > > Now it reads the handle, increments it, then decrements it. > > > > > > y = (x + 1) - 1 = x > > > > This code can be executed concurrently on multiple CPUs. We want to ensure > > that each call to > > establish gpadl gets a unique local handle. The earlier code was buggy in > > that we would read the > > handle and then atomically increment it. Thus, multiple CPUs could read the > > identical current > > value which would be their local handle. What we want is the ability to > > atomically read and increment > > the value - this would ensure that each caller got a unique value even if > > they executed the code > > concurrently on multiple CPUs. The API atomic_inc_return(), atomically > > increments and returns the > > incremented value. We locally decrement this value to emulate the logic of > > "read the current value and > > atomically increment the value. > > > > Hope this helps, > > > > K. Y > > > > [...] > > So to avoid concurrency issues you used a single atomic operation > instead of two separate operations. That make sense. But it still > doesn't explain why you changed the calculation by subtracting 1. The calculation appears identical to my reading, the original form was: next_gpadl_handle = atomic_read(&vmbus_connection.next_gpadl_handle); atomic_inc(&vmbus_connection.next_gpadl_handle); or: y = x; x++; so y == x' (x before incrementing) the new code is: next_gpadl_handle = (atomic_inc_return(&vmbus_connection.next_gpadl_handle) - 1); or: y = ++x - 1; Also making y = x' (x before incrementing) -apw ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] Staging driver patches for 3.19-rc1
On Mon, Dec 15, 2014 at 6:55 PM, Greg KH wrote: > The following changes since commit 009d0431c3914de64666bec0d350e54fdd59df6a: > > Linux 3.18-rc7 (2014-11-30 16:42:27 -0800) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ > tags/staging-3.19-rc1 > > for you to fetch changes up to 17d2c6439be65777245914be354c5a97c76ad246: > > Staging: slicoss: Fix long line issues in slicoss.c (2014-12-02 16:54:43 > -0800) > > > Staging patches for 3.19-rc1 > > Here's the big staging tree pull request for 3.19-rc1. > > We continued to delete more lines than were added, always a good thing, > but not at a huge rate this release, only about 70k lines removed > overall mostly from removing the horrid bcm driver. > > Lots of normal staging driver cleanups and fixes all over the place, > well over a thousand of them, the shortlog shows all the horrid details. > > The "contentious" thing here is the movement of the Android binder code > out of staging into the "real" part of the kernel. This is code that > has been stable for a few years now and is working as-is in the tens of > millions of devices with no issues. Yes, the code is horrid, and the > userspace api leaves a lot to be desired, but it's not going to change > due to legacy issues that we have no control over. Because so many > devices and companies rely on this, and the code is stable, might as > well promote it out of staging. > > This was all discussed at the Linux Plumbers conference, and everyone > participating agreed that this was the best way forward. > > There is work happening to replace the binder code with something new > that is happening right now, but I don't expect to see the results of > that work for another year at the earliest. If that ever happens, and > Android switches over to it, I'll gladly remove this version I don't understand this kind of logic. a) Binder is considered a piece of shite. b) Google is working on a (hopefully sane) replacement. Why moving it out of staging then? What is the benefit? Keep it there for more 2-3 years and then remove it. If you move it now out of staging into the core kernel it will be considered ABI and getting rid of it can be hard... -- Thanks, //richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] Staging driver patches for 3.19-rc1
On Mon, Dec 15, 2014 at 07:23:35PM +0100, Richard Weinberger wrote: > On Mon, Dec 15, 2014 at 6:55 PM, Greg KH wrote: > > The following changes since commit 009d0431c3914de64666bec0d350e54fdd59df6a: > > > > Linux 3.18-rc7 (2014-11-30 16:42:27 -0800) > > > > are available in the git repository at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ > > tags/staging-3.19-rc1 > > > > for you to fetch changes up to 17d2c6439be65777245914be354c5a97c76ad246: > > > > Staging: slicoss: Fix long line issues in slicoss.c (2014-12-02 16:54:43 > > -0800) > > > > > > Staging patches for 3.19-rc1 > > > > Here's the big staging tree pull request for 3.19-rc1. > > > > We continued to delete more lines than were added, always a good thing, > > but not at a huge rate this release, only about 70k lines removed > > overall mostly from removing the horrid bcm driver. > > > > Lots of normal staging driver cleanups and fixes all over the place, > > well over a thousand of them, the shortlog shows all the horrid details. > > > > The "contentious" thing here is the movement of the Android binder code > > out of staging into the "real" part of the kernel. This is code that > > has been stable for a few years now and is working as-is in the tens of > > millions of devices with no issues. Yes, the code is horrid, and the > > userspace api leaves a lot to be desired, but it's not going to change > > due to legacy issues that we have no control over. Because so many > > devices and companies rely on this, and the code is stable, might as > > well promote it out of staging. > > > > This was all discussed at the Linux Plumbers conference, and everyone > > participating agreed that this was the best way forward. > > > > There is work happening to replace the binder code with something new > > that is happening right now, but I don't expect to see the results of > > that work for another year at the earliest. If that ever happens, and > > Android switches over to it, I'll gladly remove this version > > I don't understand this kind of logic. > a) Binder is considered a piece of shite. A piece of "shite" that works for the domain it is in, and people rely on it. > b) Google is working on a (hopefully sane) replacement. I never said that Google was the one working on a replacement. > Why moving it out of staging then? What is the benefit? > Keep it there for more 2-3 years and then remove it. Because code in staging either has to progress forward to be merged out of staging, or it gets deleted. Just leaving it in staging for 2-4 more years doesn't mean anything different from moving it to drivers/android/, if I'm still maintaining it, right? What it does say is that people rely on this thing, probably you do as well, so let's mark it as such. > If you move it now out of staging into the core kernel it will be considered > ABI > and getting rid of it can be hard... It's already considered an "ABI" and removing it is hard, nothing has changed there. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] Staging driver patches for 3.19-rc1
On Mon, Dec 15, 2014 at 07:23:35PM +0100, Richard Weinberger wrote: > I don't understand this kind of logic. > a) Binder is considered a piece of shite. > b) Google is working on a (hopefully sane) replacement. > > Why moving it out of staging then? What is the benefit? There is none, and Greg didn't even bother addressing the various comments when this first came up. So a clear NAK from me on this one. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] Staging driver patches for 3.19-rc1
On Mon, Dec 15, 2014 at 10:39:15AM -0800, Christoph Hellwig wrote: > On Mon, Dec 15, 2014 at 07:23:35PM +0100, Richard Weinberger wrote: > > I don't understand this kind of logic. > > a) Binder is considered a piece of shite. > > b) Google is working on a (hopefully sane) replacement. > > > > Why moving it out of staging then? What is the benefit? > > There is none, and Greg didn't even bother addressing the various > comments when this first came up. I thought I did, it was a long thread at the time, and I was on the road for 3 weeks, sorry if I missed something. > So a clear NAK from me on this one. You don't have to maintain it, I do, so why does it concern you? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] Staging driver patches for 3.19-rc1
Am 15.12.2014 um 19:30 schrieb Greg KH: > On Mon, Dec 15, 2014 at 07:23:35PM +0100, Richard Weinberger wrote: >> On Mon, Dec 15, 2014 at 6:55 PM, Greg KH wrote: >>> The following changes since commit 009d0431c3914de64666bec0d350e54fdd59df6a: >>> >>> Linux 3.18-rc7 (2014-11-30 16:42:27 -0800) >>> >>> are available in the git repository at: >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ >>> tags/staging-3.19-rc1 >>> >>> for you to fetch changes up to 17d2c6439be65777245914be354c5a97c76ad246: >>> >>> Staging: slicoss: Fix long line issues in slicoss.c (2014-12-02 16:54:43 >>> -0800) >>> >>> >>> Staging patches for 3.19-rc1 >>> >>> Here's the big staging tree pull request for 3.19-rc1. >>> >>> We continued to delete more lines than were added, always a good thing, >>> but not at a huge rate this release, only about 70k lines removed >>> overall mostly from removing the horrid bcm driver. >>> >>> Lots of normal staging driver cleanups and fixes all over the place, >>> well over a thousand of them, the shortlog shows all the horrid details. >>> >>> The "contentious" thing here is the movement of the Android binder code >>> out of staging into the "real" part of the kernel. This is code that >>> has been stable for a few years now and is working as-is in the tens of >>> millions of devices with no issues. Yes, the code is horrid, and the >>> userspace api leaves a lot to be desired, but it's not going to change >>> due to legacy issues that we have no control over. Because so many >>> devices and companies rely on this, and the code is stable, might as >>> well promote it out of staging. >>> >>> This was all discussed at the Linux Plumbers conference, and everyone >>> participating agreed that this was the best way forward. >>> >>> There is work happening to replace the binder code with something new >>> that is happening right now, but I don't expect to see the results of >>> that work for another year at the earliest. If that ever happens, and >>> Android switches over to it, I'll gladly remove this version >> >> I don't understand this kind of logic. >> a) Binder is considered a piece of shite. > > A piece of "shite" that works for the domain it is in, and people rely > on it. Using this argument we could merge every singe vendor tree too. The crap they carry works for their domain too... ;-) >> b) Google is working on a (hopefully sane) replacement. > > I never said that Google was the one working on a replacement. Okay. Who is working on it? Is there a change that Android will pick it up? >> Why moving it out of staging then? What is the benefit? >> Keep it there for more 2-3 years and then remove it. > > Because code in staging either has to progress forward to be merged out > of staging, or it gets deleted. Just leaving it in staging for 2-4 more > years doesn't mean anything different from moving it to > drivers/android/, if I'm still maintaining it, right? What it does say > is that people rely on this thing, probably you do as well, so let's > mark it as such. > >> If you move it now out of staging into the core kernel it will be considered >> ABI >> and getting rid of it can be hard... > > It's already considered an "ABI" and removing it is hard, nothing has > changed there. Since when is stuff in staging considered ABI? Thanks, //richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] Staging driver patches for 3.19-rc1
On Mon, Dec 15, 2014 at 07:36:00PM +0100, Richard Weinberger wrote: > > Am 15.12.2014 um 19:30 schrieb Greg KH: > > On Mon, Dec 15, 2014 at 07:23:35PM +0100, Richard Weinberger wrote: > >> On Mon, Dec 15, 2014 at 6:55 PM, Greg KH > >> wrote: > >>> The following changes since commit > >>> 009d0431c3914de64666bec0d350e54fdd59df6a: > >>> > >>> Linux 3.18-rc7 (2014-11-30 16:42:27 -0800) > >>> > >>> are available in the git repository at: > >>> > >>> git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ > >>> tags/staging-3.19-rc1 > >>> > >>> for you to fetch changes up to 17d2c6439be65777245914be354c5a97c76ad246: > >>> > >>> Staging: slicoss: Fix long line issues in slicoss.c (2014-12-02 > >>> 16:54:43 -0800) > >>> > >>> > >>> Staging patches for 3.19-rc1 > >>> > >>> Here's the big staging tree pull request for 3.19-rc1. > >>> > >>> We continued to delete more lines than were added, always a good thing, > >>> but not at a huge rate this release, only about 70k lines removed > >>> overall mostly from removing the horrid bcm driver. > >>> > >>> Lots of normal staging driver cleanups and fixes all over the place, > >>> well over a thousand of them, the shortlog shows all the horrid details. > >>> > >>> The "contentious" thing here is the movement of the Android binder code > >>> out of staging into the "real" part of the kernel. This is code that > >>> has been stable for a few years now and is working as-is in the tens of > >>> millions of devices with no issues. Yes, the code is horrid, and the > >>> userspace api leaves a lot to be desired, but it's not going to change > >>> due to legacy issues that we have no control over. Because so many > >>> devices and companies rely on this, and the code is stable, might as > >>> well promote it out of staging. > >>> > >>> This was all discussed at the Linux Plumbers conference, and everyone > >>> participating agreed that this was the best way forward. > >>> > >>> There is work happening to replace the binder code with something new > >>> that is happening right now, but I don't expect to see the results of > >>> that work for another year at the earliest. If that ever happens, and > >>> Android switches over to it, I'll gladly remove this version > >> > >> I don't understand this kind of logic. > >> a) Binder is considered a piece of shite. > > > > A piece of "shite" that works for the domain it is in, and people rely > > on it. > > Using this argument we could merge every singe vendor tree too. > The crap they carry works for their domain too... ;-) That's a false-argument, you know that. This code falls into the "distros have been using it and it is proven to work" requirement that we have often made for new features. Fact is, this is useful code, in this area. In the domain it is used in, it works properly, and the abi is sufficient. Yes, it's ugly in spaces, and insecure if used outside of Android, but that's ok for the users of the code. > >> b) Google is working on a (hopefully sane) replacement. > > > > I never said that Google was the one working on a replacement. > > Okay. Who is working on it? Some other company, it's not my place to pre-announce projects, sorry. > Is there a change that Android will pick it up? Yes. > >> Why moving it out of staging then? What is the benefit? > >> Keep it there for more 2-3 years and then remove it. > > > > Because code in staging either has to progress forward to be merged out > > of staging, or it gets deleted. Just leaving it in staging for 2-4 more > > years doesn't mean anything different from moving it to > > drivers/android/, if I'm still maintaining it, right? What it does say > > is that people rely on this thing, probably you do as well, so let's > > mark it as such. > > > >> If you move it now out of staging into the core kernel it will be > >> considered ABI > >> and getting rid of it can be hard... > > > > It's already considered an "ABI" and removing it is hard, nothing has > > changed there. > > Since when is stuff in staging considered ABI? Since a few hundred million devices use it and we have userspace code that relies on it and can't be changed? thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V3 1/1] Drivers: hv: vmbus: Fix a bug in vmbus_establish_gpadl()
Andy, On Mon, Dec 15, 2014 at 03:47:04PM +, Andy Whitcroft wrote: > On Sun, Dec 14, 2014 at 11:59:19PM -0800, Jeremiah Mahler wrote: [...] > > The calculation appears identical to my reading, the original form was: > > next_gpadl_handle = atomic_read(&vmbus_connection.next_gpadl_handle); > atomic_inc(&vmbus_connection.next_gpadl_handle); > > or: > > y = x; > x++; > > so y == x' (x before incrementing) > > the new code is: > > next_gpadl_handle = (atomic_inc_return(&vmbus_connection.next_gpadl_handle) > - 1); > > or: > > y = ++x - 1; > > Also making y = x' (x before incrementing) > > -apw Ah, you are right. The increment before/after messed me up. Thanks for clearing that up for me :-) -- - Jeremiah Mahler ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] Staging driver patches for 3.19-rc1
On Mon, Dec 15, 2014 at 10:41:03AM -0800, Greg KH wrote: > On Mon, Dec 15, 2014 at 10:39:15AM -0800, Christoph Hellwig wrote: > > On Mon, Dec 15, 2014 at 07:23:35PM +0100, Richard Weinberger wrote: > > > I don't understand this kind of logic. > > > a) Binder is considered a piece of shite. > > > b) Google is working on a (hopefully sane) replacement. > > > > > > Why moving it out of staging then? What is the benefit? > > > > There is none, and Greg didn't even bother addressing the various > > comments when this first came up. > > I thought I did, it was a long thread at the time, and I was on the road > for 3 weeks, sorry if I missed something. > > > So a clear NAK from me on this one. > > You don't have to maintain it, I do, so why does it concern you? Ok, that was a bit snotty on my part, I apologize. But really, this is self-contained, doesn't touch any core infrastructure, and is really just like any other driver for hardware that people don't use. It shouldn't affect anything elsewhere in the kernel, so objecting to it seems odd to me. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] Staging driver patches for 3.19-rc1
Am 15.12.2014 um 19:44 schrieb Greg KH: > On Mon, Dec 15, 2014 at 07:36:00PM +0100, Richard Weinberger wrote: >> >> Am 15.12.2014 um 19:30 schrieb Greg KH: >>> On Mon, Dec 15, 2014 at 07:23:35PM +0100, Richard Weinberger wrote: On Mon, Dec 15, 2014 at 6:55 PM, Greg KH wrote: > The following changes since commit > 009d0431c3914de64666bec0d350e54fdd59df6a: > > Linux 3.18-rc7 (2014-11-30 16:42:27 -0800) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git/ > tags/staging-3.19-rc1 > > for you to fetch changes up to 17d2c6439be65777245914be354c5a97c76ad246: > > Staging: slicoss: Fix long line issues in slicoss.c (2014-12-02 > 16:54:43 -0800) > > > Staging patches for 3.19-rc1 > > Here's the big staging tree pull request for 3.19-rc1. > > We continued to delete more lines than were added, always a good thing, > but not at a huge rate this release, only about 70k lines removed > overall mostly from removing the horrid bcm driver. > > Lots of normal staging driver cleanups and fixes all over the place, > well over a thousand of them, the shortlog shows all the horrid details. > > The "contentious" thing here is the movement of the Android binder code > out of staging into the "real" part of the kernel. This is code that > has been stable for a few years now and is working as-is in the tens of > millions of devices with no issues. Yes, the code is horrid, and the > userspace api leaves a lot to be desired, but it's not going to change > due to legacy issues that we have no control over. Because so many > devices and companies rely on this, and the code is stable, might as > well promote it out of staging. > > This was all discussed at the Linux Plumbers conference, and everyone > participating agreed that this was the best way forward. > > There is work happening to replace the binder code with something new > that is happening right now, but I don't expect to see the results of > that work for another year at the earliest. If that ever happens, and > Android switches over to it, I'll gladly remove this version I don't understand this kind of logic. a) Binder is considered a piece of shite. >>> >>> A piece of "shite" that works for the domain it is in, and people rely >>> on it. >> >> Using this argument we could merge every singe vendor tree too. >> The crap they carry works for their domain too... ;-) > > That's a false-argument, you know that. This code falls into the > "distros have been using it and it is proven to work" requirement that > we have often made for new features. > > Fact is, this is useful code, in this area. In the domain it is used > in, it works properly, and the abi is sufficient. Yes, it's ugly in > spaces, and insecure if used outside of Android, but that's ok for the > users of the code. Let's discuss this while having a few beers. It is going to be philosophic. b) Google is working on a (hopefully sane) replacement. >>> >>> I never said that Google was the one working on a replacement. >> >> Okay. Who is working on it? > > Some other company, it's not my place to pre-announce projects, sorry. Yeah, this is sad. :-\ >> Is there a change that Android will pick it up? > > Yes. So then wait until this happens and ignore binder. Why moving it out of staging then? What is the benefit? Keep it there for more 2-3 years and then remove it. >>> >>> Because code in staging either has to progress forward to be merged out >>> of staging, or it gets deleted. Just leaving it in staging for 2-4 more >>> years doesn't mean anything different from moving it to >>> drivers/android/, if I'm still maintaining it, right? What it does say >>> is that people rely on this thing, probably you do as well, so let's >>> mark it as such. >>> If you move it now out of staging into the core kernel it will be considered ABI and getting rid of it can be hard... >>> >>> It's already considered an "ABI" and removing it is hard, nothing has >>> changed there. >> >> Since when is stuff in staging considered ABI? > > Since a few hundred million devices use it and we have userspace code > that relies on it and can't be changed? It is news to me that these devices use a mainline kernel. I'm well a aware of the fact that there are a lot of android devices out there. But why moving binder into the core kernel? What is the benefit? Does Google even care? Thanks, //richard ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: bcm: fix spaces before commas in Bcmchar.c
This is a patch to the Bcmchar.c file that fixes a space before comma warning
found by checkpatch.pl.
Signed-off-by: Jack Wilson
---
drivers/staging/bcm/Bcmchar.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/bcm/Bcmchar.c b/drivers/staging/bcm/Bcmchar.c
index 88ce2da..37a184a 100644
--- a/drivers/staging/bcm/Bcmchar.c
+++ b/drivers/staging/bcm/Bcmchar.c
@@ -1647,7 +1647,7 @@ static int bcm_char_ioctl_flash2x_section_read(void
__user *argp,
read_offset = flash_2x_read.offset;
OutPutBuff = io_buff.OutputBuffer;
- read_buff = kzalloc(buff_size , GFP_KERNEL);
+ read_buff = kzalloc(buff_size, GFP_KERNEL);
if (read_buff == NULL) {
BCM_DEBUG_PRINT(ad, DBG_TYPE_PRINTK, 0, 0,
@@ -2192,7 +2192,7 @@ static int bcm_char_ioctl_nvm_raw_read(void __user *argp,
read_offset = nvm_read.uiOffset;
OutPutBuff = nvm_read.pBuffer;
- read_buff = kzalloc(buff_size , GFP_KERNEL);
+ read_buff = kzalloc(buff_size, GFP_KERNEL);
if (read_buff == NULL) {
BCM_DEBUG_PRINT(ad, DBG_TYPE_PRINTK, 0, 0,
"Memory allocation failed for Flash 2.x Read
Structure");
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] Drivers: hv: vmbus: Support a vmbus API for efficiently sending page arrays
Currently, the API for sending a multi-page buffer over VMBUS is limited to
a maximum pfn array of MAX_MULTIPAGE_BUFFER_COUNT. This limitation is
not imposed by the host and unnecessarily limits the maximum payload
that can be sent. Implement an API that does not have this restriction.
Signed-off-by: K. Y. Srinivasan
---
drivers/hv/channel.c | 44
include/linux/hyperv.h | 31 +++
2 files changed, 75 insertions(+), 0 deletions(-)
diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
index c76ffbe..18c4f23 100644
--- a/drivers/hv/channel.c
+++ b/drivers/hv/channel.c
@@ -686,6 +686,50 @@ EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer);
/*
* vmbus_sendpacket_multipagebuffer - Send a multi-page buffer packet
* using a GPADL Direct packet type.
+ * The buffer includes the vmbus descriptor.
+ */
+int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
+ struct vmbus_packet_mpb_array *desc,
+ u32 desc_size,
+ void *buffer, u32 bufferlen, u64 requestid)
+{
+ int ret;
+ u32 packetlen;
+ u32 packetlen_aligned;
+ struct kvec bufferlist[3];
+ u64 aligned_data = 0;
+ bool signal = false;
+
+ packetlen = desc_size + bufferlen;
+ packetlen_aligned = ALIGN(packetlen, sizeof(u64));
+
+ /* Setup the descriptor */
+ desc->type = VM_PKT_DATA_USING_GPA_DIRECT;
+ desc->flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
+ desc->dataoffset8 = desc_size >> 3; /* in 8-bytes grandularity */
+ desc->length8 = (u16)(packetlen_aligned >> 3);
+ desc->transactionid = requestid;
+ desc->rangecount = 1;
+
+ bufferlist[0].iov_base = desc;
+ bufferlist[0].iov_len = desc_size;
+ bufferlist[1].iov_base = buffer;
+ bufferlist[1].iov_len = bufferlen;
+ bufferlist[2].iov_base = &aligned_data;
+ bufferlist[2].iov_len = (packetlen_aligned - packetlen);
+
+ ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, &signal);
+
+ if (ret == 0 && signal)
+ vmbus_setevent(channel);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(vmbus_sendpacket_mpb_desc);
+
+/*
+ * vmbus_sendpacket_multipagebuffer - Send a multi-page buffer packet
+ * using a GPADL Direct packet type.
*/
int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel,
struct hv_multipage_buffer *multi_pagebuffer,
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 08cfaff..8615b0d 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -57,6 +57,18 @@ struct hv_multipage_buffer {
u64 pfn_array[MAX_MULTIPAGE_BUFFER_COUNT];
};
+/*
+ * Multiple-page buffer array; the pfn array is variable size:
+ * The number of entries in the PFN array is determined by
+ * "len" and "offset".
+ */
+struct hv_mpb_array {
+ /* Length and Offset determines the # of pfns in the array */
+ u32 len;
+ u32 offset;
+ u64 pfn_array[];
+};
+
/* 0x18 includes the proprietary packet header */
#define MAX_PAGE_BUFFER_PACKET (0x18 + \
(sizeof(struct hv_page_buffer) * \
@@ -812,6 +824,18 @@ struct vmbus_channel_packet_multipage_buffer {
struct hv_multipage_buffer range;
} __packed;
+/* The format must be the same as struct vmdata_gpa_direct */
+struct vmbus_packet_mpb_array {
+ u16 type;
+ u16 dataoffset8;
+ u16 length8;
+ u16 flags;
+ u64 transactionid;
+ u32 reserved;
+ u32 rangecount; /* Always 1 in this case */
+ struct hv_mpb_array range;
+} __packed;
+
extern int vmbus_open(struct vmbus_channel *channel,
u32 send_ringbuffersize,
@@ -843,6 +867,13 @@ extern int vmbus_sendpacket_multipagebuffer(struct
vmbus_channel *channel,
u32 bufferlen,
u64 requestid);
+extern int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
+struct vmbus_packet_mpb_array *mpb,
+u32 desc_size,
+void *buffer,
+u32 bufferlen,
+u64 requestid);
+
extern int vmbus_establish_gpadl(struct vmbus_channel *channel,
void *kbuffer,
u32 size,
--
1.7.4.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [GIT PULL] Staging driver patches for 3.19-rc1
On Mon, Dec 15, 2014 at 08:04:53PM +0100, Richard Weinberger wrote: > > Fact is, this is useful code, in this area. In the domain it is used > > in, it works properly, and the abi is sufficient. Yes, it's ugly in > > spaces, and insecure if used outside of Android, but that's ok for the > > users of the code. > > Let's discuss this while having a few beers. > It is going to be philosophic. I'm all for beers and philosopic talk, sounds good :) > >> Is there a change that Android will pick it up? > > > > Yes. > > So then wait until this happens and ignore binder. I have been, for years now. But the issue is, drivers/staging/ should not just be a "dumping ground" for code. Code in there needs to either move forward in being cleaned up and accepted, or it needs to be deleted. This is the reason why I deleted the binder code from the tree a few years ago. Turns out, that's the first commit vendors revert when they make an android product. Then they add on a mis-match of patches on top of it, bringing the code kind-of-up-to-date with the newer kernel, and ship it. That has caused problems by people not knowing what to apply and fix in order to handle abi changes. So the code went back into staging, and got fixed up, and now that's as far as I can take it without either deleting it, or moving it out of staging. So I'm trying to move it out of staging. Now I know this is a "special case" for stuff that might be "ugly". Fact is, we are stuck with this user api due to ignoring this code for many years on the community side, as well as Google not taking the time and effort originally to push it upstream. There is fault on both sides here, I agree. And because of that, I'm willing to maintain this on our side. I have confirmation that Google employees will also co-maintain it as well. But the userspace api is something that we just have to live with, as ugly as it is, until someone, who has the ability to commit to the Android userspace side, does the work. > > Since a few hundred million devices use it and we have userspace code > > that relies on it and can't be changed? > > It is news to me that these devices use a mainline kernel. They always start with a mainline kernel, and then usually add bsp support on top of it. The non-bsp code in the Android tree these days is very low. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: bcm: fix spaces before commas in Bcmchar.c
On Mon, Dec 15, 2014 at 11:22:29AM -0800, Jack Wilson wrote: > This is a patch to the Bcmchar.c file that fixes a space before comma warning > found by checkpatch.pl. > Signed-off-by: Jack Wilson > --- > drivers/staging/bcm/Bcmchar.c | 4 ++-- Odd, that file is gone from my tree, I have no idea what tree you are working against here... ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] Drivers: hv: vmbus: Support a vmbus API for efficiently sending page arrays
On Mon, Dec 15, 2014 at 12:33:47PM -0800, K. Y. Srinivasan wrote:
> Currently, the API for sending a multi-page buffer over VMBUS is limited to
> a maximum pfn array of MAX_MULTIPAGE_BUFFER_COUNT. This limitation is
> not imposed by the host and unnecessarily limits the maximum payload
> that can be sent. Implement an API that does not have this restriction.
>
> Signed-off-by: K. Y. Srinivasan
> ---
> drivers/hv/channel.c | 44
> include/linux/hyperv.h | 31 +++
> 2 files changed, 75 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index c76ffbe..18c4f23 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
> @@ -686,6 +686,50 @@ EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer);
> /*
> * vmbus_sendpacket_multipagebuffer - Send a multi-page buffer packet
> * using a GPADL Direct packet type.
> + * The buffer includes the vmbus descriptor.
> + */
> +int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
> + struct vmbus_packet_mpb_array *desc,
> + u32 desc_size,
> + void *buffer, u32 bufferlen, u64 requestid)
> +{
> + int ret;
> + u32 packetlen;
> + u32 packetlen_aligned;
> + struct kvec bufferlist[3];
> + u64 aligned_data = 0;
> + bool signal = false;
> +
> + packetlen = desc_size + bufferlen;
> + packetlen_aligned = ALIGN(packetlen, sizeof(u64));
> +
> + /* Setup the descriptor */
> + desc->type = VM_PKT_DATA_USING_GPA_DIRECT;
> + desc->flags = VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
> + desc->dataoffset8 = desc_size >> 3; /* in 8-bytes grandularity */
> + desc->length8 = (u16)(packetlen_aligned >> 3);
> + desc->transactionid = requestid;
> + desc->rangecount = 1;
> +
> + bufferlist[0].iov_base = desc;
> + bufferlist[0].iov_len = desc_size;
> + bufferlist[1].iov_base = buffer;
> + bufferlist[1].iov_len = bufferlen;
> + bufferlist[2].iov_base = &aligned_data;
> + bufferlist[2].iov_len = (packetlen_aligned - packetlen);
> +
> + ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3, &signal);
> +
> + if (ret == 0 && signal)
> + vmbus_setevent(channel);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(vmbus_sendpacket_mpb_desc);
> +
> +/*
> + * vmbus_sendpacket_multipagebuffer - Send a multi-page buffer packet
> + * using a GPADL Direct packet type.
> */
> int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel,
> struct hv_multipage_buffer *multi_pagebuffer,
> diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
> index 08cfaff..8615b0d 100644
> --- a/include/linux/hyperv.h
> +++ b/include/linux/hyperv.h
> @@ -57,6 +57,18 @@ struct hv_multipage_buffer {
> u64 pfn_array[MAX_MULTIPAGE_BUFFER_COUNT];
> };
>
> +/*
> + * Multiple-page buffer array; the pfn array is variable size:
> + * The number of entries in the PFN array is determined by
> + * "len" and "offset".
> + */
> +struct hv_mpb_array {
> + /* Length and Offset determines the # of pfns in the array */
> + u32 len;
> + u32 offset;
> + u64 pfn_array[];
> +};
Does this cross the user/kernel boundry? If so, they need to be __u32
and __u64 variables.
> +
> /* 0x18 includes the proprietary packet header */
> #define MAX_PAGE_BUFFER_PACKET (0x18 + \
> (sizeof(struct hv_page_buffer) * \
> @@ -812,6 +824,18 @@ struct vmbus_channel_packet_multipage_buffer {
> struct hv_multipage_buffer range;
> } __packed;
>
> +/* The format must be the same as struct vmdata_gpa_direct */
> +struct vmbus_packet_mpb_array {
> + u16 type;
> + u16 dataoffset8;
> + u16 length8;
> + u16 flags;
> + u64 transactionid;
> + u32 reserved;
> + u32 rangecount; /* Always 1 in this case */
> + struct hv_mpb_array range;
> +} __packed;
Same here.
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH 1/1] Drivers: hv: vmbus: Support a vmbus API for efficiently sending page arrays
> -Original Message-
> From: Greg KH [mailto:gre...@linuxfoundation.org]
> Sent: Monday, December 15, 2014 11:34 AM
> To: KY Srinivasan
> Cc: linux-ker...@vger.kernel.org; de...@linuxdriverproject.org;
> o...@aepfle.de; a...@canonical.com; jasow...@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: hv: vmbus: Support a vmbus API for
> efficiently sending page arrays
>
> On Mon, Dec 15, 2014 at 12:33:47PM -0800, K. Y. Srinivasan wrote:
> > Currently, the API for sending a multi-page buffer over VMBUS is
> > limited to a maximum pfn array of MAX_MULTIPAGE_BUFFER_COUNT.
> This
> > limitation is not imposed by the host and unnecessarily limits the
> > maximum payload that can be sent. Implement an API that does not have
> this restriction.
> >
> > Signed-off-by: K. Y. Srinivasan
> > ---
> > drivers/hv/channel.c | 44
>
> > include/linux/hyperv.h | 31 +++
> > 2 files changed, 75 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index
> > c76ffbe..18c4f23 100644
> > --- a/drivers/hv/channel.c
> > +++ b/drivers/hv/channel.c
> > @@ -686,6 +686,50 @@
> EXPORT_SYMBOL_GPL(vmbus_sendpacket_pagebuffer);
> > /*
> > * vmbus_sendpacket_multipagebuffer - Send a multi-page buffer packet
> > * using a GPADL Direct packet type.
> > + * The buffer includes the vmbus descriptor.
> > + */
> > +int vmbus_sendpacket_mpb_desc(struct vmbus_channel *channel,
> > + struct vmbus_packet_mpb_array *desc,
> > + u32 desc_size,
> > + void *buffer, u32 bufferlen, u64 requestid) {
> > + int ret;
> > + u32 packetlen;
> > + u32 packetlen_aligned;
> > + struct kvec bufferlist[3];
> > + u64 aligned_data = 0;
> > + bool signal = false;
> > +
> > + packetlen = desc_size + bufferlen;
> > + packetlen_aligned = ALIGN(packetlen, sizeof(u64));
> > +
> > + /* Setup the descriptor */
> > + desc->type = VM_PKT_DATA_USING_GPA_DIRECT;
> > + desc->flags =
> VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED;
> > + desc->dataoffset8 = desc_size >> 3; /* in 8-bytes grandularity */
> > + desc->length8 = (u16)(packetlen_aligned >> 3);
> > + desc->transactionid = requestid;
> > + desc->rangecount = 1;
> > +
> > + bufferlist[0].iov_base = desc;
> > + bufferlist[0].iov_len = desc_size;
> > + bufferlist[1].iov_base = buffer;
> > + bufferlist[1].iov_len = bufferlen;
> > + bufferlist[2].iov_base = &aligned_data;
> > + bufferlist[2].iov_len = (packetlen_aligned - packetlen);
> > +
> > + ret = hv_ringbuffer_write(&channel->outbound, bufferlist, 3,
> > +&signal);
> > +
> > + if (ret == 0 && signal)
> > + vmbus_setevent(channel);
> > +
> > + return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(vmbus_sendpacket_mpb_desc);
> > +
> > +/*
> > + * vmbus_sendpacket_multipagebuffer - Send a multi-page buffer packet
> > + * using a GPADL Direct packet type.
> > */
> > int vmbus_sendpacket_multipagebuffer(struct vmbus_channel *channel,
> > struct hv_multipage_buffer
> *multi_pagebuffer, diff --git
> > a/include/linux/hyperv.h b/include/linux/hyperv.h index
> > 08cfaff..8615b0d 100644
> > --- a/include/linux/hyperv.h
> > +++ b/include/linux/hyperv.h
> > @@ -57,6 +57,18 @@ struct hv_multipage_buffer {
> > u64 pfn_array[MAX_MULTIPAGE_BUFFER_COUNT];
> > };
> >
> > +/*
> > + * Multiple-page buffer array; the pfn array is variable size:
> > + * The number of entries in the PFN array is determined by
> > + * "len" and "offset".
> > + */
> > +struct hv_mpb_array {
> > + /* Length and Offset determines the # of pfns in the array */
> > + u32 len;
> > + u32 offset;
> > + u64 pfn_array[];
> > +};
>
> Does this cross the user/kernel boundry? If so, they need to be __u32 and
> __u64 variables.
This does not cross user/kernel boundry.
>
> > +
> > /* 0x18 includes the proprietary packet header */
> > #define MAX_PAGE_BUFFER_PACKET (0x18 +
> \
> > (sizeof(struct hv_page_buffer) * \
> @@ -812,6 +824,18 @@ struct
> > vmbus_channel_packet_multipage_buffer {
> > struct hv_multipage_buffer range;
> > } __packed;
> >
> > +/* The format must be the same as struct vmdata_gpa_direct */ struct
> > +vmbus_packet_mpb_array {
> > + u16 type;
> > + u16 dataoffset8;
> > + u16 length8;
> > + u16 flags;
> > + u64 transactionid;
> > + u32 reserved;
> > + u32 rangecount; /* Always 1 in this case */
> > + struct hv_mpb_array range;
> > +} __packed;
>
> Same here.
This is completely internal to the kernel.
Regards,
K. Y
>
> thanks,
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: wlan-ng: hfa384x_usb: fixed an 'else' statement coding style issue
Thank you for the quick feedback.
It was my first patch to the kernel and I wanted to be sure it would get right
to the community.
I'll be making a version two with the consideration you brought me.
Thank you again,
Best regards,
Eduardo Barretto
On Sun, Dec 14, 2014 at 06:11:30PM -0800, Joe Perches wrote:
> On Mon, 2014-12-15 at 00:02 -0200, Eduardo Barretto wrote:
> > Fixed a coding style issue
> []
> > diff --git a/drivers/staging/wlan-ng/hfa384x_usb.c
> > b/drivers/staging/wlan-ng/hfa384x_usb.c
> []
> > @@ -4123,12 +4123,11 @@ static int hfa384x_isgood_pdrcode(u16 pdrcode)
> > pr_debug("Encountered unknown PDR#=0x%04x, assuming
> > it's ok.\n",
> > pdrcode);
> > return 1;
> > - } else {
> > - /* bad code */
> > - pr_debug("Encountered unknown PDR#=0x%04x, (>=0x1000),
> > assuming it's bad.\n",
> > -pdrcode);
> > - return 0;
> > }
> > + /* bad code */
> > + pr_debug("Encountered unknown PDR#=0x%04x, (>=0x1000), assuming
> > it's bad.\n",
> > +pdrcode);
> > + return 0;
> > }
> > return 0; /* avoid compiler warnings */
> > }
>
> While this patch isn't necessary, if any change is
> done, it might better to not have two consecutive
> return 0; uses.
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: wlan-ng: hfa384x_usb: fixed an 'else' statement coding style issue
On Mon, 2014-12-15 at 18:53 -0200, Eduardo Barretto wrote:
> Thank you for the quick feedback.
> It was my first patch to the kernel and I wanted to be sure it would get
> right to the community.
> I'll be making a version two with the consideration you brought me.
the code today is:
{
switch (prdcode) {
case [...]
return 1;
default:
if (prdcode < 0x1000) {
printk(msg1);
return 1;
else
printk(msg2);
return 0;
}
return 0; /* avoid compiler noise */
}
I think this code does not needs changing.
I think more modern compilers don't even warn
when the last return 0; isn't there.
If it were to be changed, I'd probably write it like:
{
switch (prdcode) {
case [...]
return 1;
default:
if (prdcode < 0x1000) {
printk(msg1);
return 1;
}
break;
}
printk(msg2);
return 0;
}
but I wouldn't bother.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: bcm: fix spaces before commas in Bcmchar.c
Hi Jack, bcm is removed from the linux kernel. You are working against the wrong tree! Please work against staging-next of gregkh for staging drivers! Ask me if you do not understand what the problem is! Feel free to ask me other questions as well! On 15-12-2014 11:22:29, Jack Wilson wrote: > This is a patch to the Bcmchar.c file that fixes a space before comma warning > found by checkpatch.pl. > Signed-off-by: Jack Wilson -- Mit freundlichen Grüßen, Kind regards, Matthias Beyer Proudly sent with mutt. Happily signed with gnupg. pgpDApKCsYCOL.pgp Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: lustre: obdclass: lprocfs_status.c: Fix for possible null pointer dereference
Hi Joe
No, it does not look like end can be NULL then.
Then remove the end != NULL instead?
...
if (end != NULL && *end == '.') {
However, I am hesitant to the tolower() I think double case is faster...?
Kind regards
Rickard Strandqvist
2014-12-15 2:51 GMT+01:00 Joe Perches :
> On Sun, 2014-12-14 at 23:52 +0100, Rickard Strandqvist wrote:
>> There is otherwise a risk of a possible null pointer dereference.
>>
>> Was largely found by using a static code analysis program called cppcheck.
>
> Perhaps the tool could use a little work.
> It's not possible for end to be NULL no?
>
> unsigned long long simple_strtoull(const char *cp, char **endp, unsigned int
> base)
> {
> unsigned long long result;
> unsigned int rv;
>
> cp = _parse_integer_fixup_radix(cp, &base);
> rv = _parse_integer(cp, base, &result);
> /* FIXME */
> cp += (rv & ~KSTRTOX_OVERFLOW);
>
> if (endp)
> *endp = (char *)cp;
>
> return result;
> }
> EXPORT_SYMBOL(simple_strtoull);
>
>
>> diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>> b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
> []
>
> Above this:
>
> whole = simple_strtoull(pbuf, &end, 10);
>
>> +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c
>> @@ -1897,17 +1897,19 @@ int lprocfs_write_frac_u64_helper(const char
>> *buffer, unsigned long count,
>> }
>>
>> units = 1;
>> - switch (*end) {
>> - case 'p': case 'P':
>> - units <<= 10;
>> - case 't': case 'T':
>> - units <<= 10;
>> - case 'g': case 'G':
>> - units <<= 10;
>> - case 'm': case 'M':
>> - units <<= 10;
>> - case 'k': case 'K':
>> - units <<= 10;
>> + if (end) {
>> + switch (*end) {
>> + case 'p': case 'P':
>> + units <<= 10;
>> + case 't': case 'T':
>> + units <<= 10;
>> + case 'g': case 'G':
>> + units <<= 10;
>> + case 'm': case 'M':
>> + units <<= 10;
>> + case 'k': case 'K':
>> + units <<= 10;
>> + }
>
> The only thing I might do is
>
> switch (tolower(*end)) {
>
> and remove the second case entry for each line
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference
Hi
No the rtw_hw_resume23a() is not used anywhere.
I also do a check of all functions that are not used, but not in the
drivers/staging, I suspected that these might be under development and
used in the future.
What do you want to do, who decides?
Kind regards
Rickard Strandqvist
2014-12-15 16:48 GMT+01:00 Jes Sorensen :
> Dan Carpenter writes:
>> On Sun, Dec 14, 2014 at 11:39:14PM +0100, Rickard Strandqvist wrote:
>>> There is otherwise a risk of a possible null pointer dereference.
>>>
>>> Was largely found by using a static code analysis program called cppcheck.
>>>
>>> Signed-off-by: Rickard Strandqvist
>>> ---
>>> drivers/staging/rtl8723au/os_dep/usb_intf.c |3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/staging/rtl8723au/os_dep/usb_intf.c
>>> b/drivers/staging/rtl8723au/os_dep/usb_intf.c
>>> index 865743e..71a6330 100644
>>> --- a/drivers/staging/rtl8723au/os_dep/usb_intf.c
>>> +++ b/drivers/staging/rtl8723au/os_dep/usb_intf.c
>>> @@ -351,10 +351,11 @@ error_exit:
>>> int rtw_hw_resume23a(struct rtw_adapter *padapter)
>>
>> That's weird. Is this function even called?
>
> [jes@ultrasam jes.git]$ find drivers/staging/rtl8723au -name \*.[ch] |xargs
> grep rtw_hw_resume
> drivers/staging/rtl8723au/include/osdep_intf.h:int rtw_hw_resume23a(struct
> rtw_adapter *padapter);
> drivers/staging/rtl8723au/os_dep/usb_intf.c:int rtw_hw_resume23a(struct
> rtw_adapter *padapter)
> drivers/staging/rtl8723au/os_dep/usb_intf.c:DBG_8723A("==>
> rtw_hw_resume23a\n");
> [jes@ultrasam jes.git]$ find drivers/staging/rtl8723au -name \*.[ch] |xargs
> grep rtw_hw_suspend
> drivers/staging/rtl8723au/include/osdep_intf.h:int rtw_hw_suspend23a(struct
> rtw_adapter *padapter);
> drivers/staging/rtl8723au/os_dep/usb_intf.c:int rtw_hw_suspend23a(struct
> rtw_adapter *padapter)
> drivers/staging/rtl8723au/os_dep/usb_intf.c:DBG_8723A("==>
> rtw_hw_suspend23a\n");
>
> A more useful patch would be one removing those two functions IMHO.
>
> Jes
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: lustre: osc: lproc_osc.c: Fix for possible null pointer dereference
Hi Dan
Quite right! Had to try it.
Do nothing then?
But you must agree that it is still ugly and confusing code.
Kind regards
Rickard Strandqvist
2014-12-15 11:25 GMT+01:00 Dan Carpenter :
> On Sun, Dec 14, 2014 at 11:37:18PM +0100, Rickard Strandqvist wrote:
>> There is otherwise a risk of a possible null pointer dereference.
>>
>> Was largely found by using a static code analysis program called cppcheck.
>>
>> Signed-off-by: Rickard Strandqvist
>> ---
>> drivers/staging/lustre/lustre/osc/lproc_osc.c |4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/lustre/lustre/osc/lproc_osc.c
>> b/drivers/staging/lustre/lustre/osc/lproc_osc.c
>> index 9f719bc..9ba6293 100644
>> --- a/drivers/staging/lustre/lustre/osc/lproc_osc.c
>> +++ b/drivers/staging/lustre/lustre/osc/lproc_osc.c
>> @@ -237,13 +237,15 @@ static ssize_t osc_cur_grant_bytes_seq_write(struct
>> file *file, const char *buff
>> size_t count, loff_t *off)
>> {
>> struct obd_device *obd = ((struct seq_file
>> *)file->private_data)->private;
>> - struct client_obd *cli = &obd->u.cli;
>> + struct client_obd *cli;
>
> This isn't really a dereference. You're just getting the address of
> obd->u.cli. So if obd is NULL then it won't crash.
>
> regards,
> dan carpenter
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: lustre: lustre: obdclass: lprocfs_status.c: Fix for possible null pointer dereference
On Mon, 2014-12-15 at 23:23 +0100, Rickard Strandqvist wrote:
> Hi Joe
Hello Rickard
> No, it does not look like end can be NULL then.
> Then remove the end != NULL instead?
> ...
> if (end != NULL && *end == '.') {
Up to you.
> However, I am hesitant to the tolower() I think double case is faster...?
I doubt code execution speed is paramount here.
Maybe see if the object code size is smaller one
way or the other.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723au: os_dep: usb_intf.c: Fix for possible null pointer dereference
On 12/15/2014 05:01 PM, Rickard Strandqvist wrote: Hi No the rtw_hw_resume23a() is not used anywhere. I also do a check of all functions that are not used, but not in the drivers/staging, I suspected that these might be under development and used in the future. What do you want to do, who decides? The original developers of this code do not include any of the Linux maintainers. If you see an unused routine, submit a patch to delete it. If a maintainer thinks it should be kept, your patch will be NACKed, but that is pnot likely. Larry ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCHv1 4/7] staging: iio: isl29028: deprecate use of isl in compatible string for isil
"isil" and "isl" prefixes are used at various locations inside the kernel
to reference Intersil corporation. This patch is part of a series fixing
those locations were "isl" is used in compatible strings to use the now
expected "isil" prefix instead (NASDAQ symbol for Intersil and most used
version). The old compatible string is kept for backward compatibility.
Signed-off-by: Arnaud Ebalard
---
drivers/staging/iio/light/isl29028.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/iio/light/isl29028.c
b/drivers/staging/iio/light/isl29028.c
index e969107ddb47..6440e3b293ca 100644
--- a/drivers/staging/iio/light/isl29028.c
+++ b/drivers/staging/iio/light/isl29028.c
@@ -537,8 +537,8 @@ static const struct i2c_device_id isl29028_id[] = {
MODULE_DEVICE_TABLE(i2c, isl29028_id);
static const struct of_device_id isl29028_of_match[] = {
- { .compatible = "isl,isl29028", },
- { .compatible = "isil,isl29028", },/* deprecated, don't use */
+ { .compatible = "isl,isl29028", }, /* for backward compat., don't use */
+ { .compatible = "isil,isl29028", },
{ },
};
MODULE_DEVICE_TABLE(of, isl29028_of_match);
--
2.1.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH] scsi:storvsc enable reading from VPD pages on SPC-2
Thanks Martin for the explanation. I'll send out another patch. > -Original Message- > From: Martin K. Petersen [mailto:martin.peter...@oracle.com] > Sent: Thursday, December 11, 2014 7:04 PM > To: Long Li > Cc: Martin K. Petersen; KY Srinivasan; Haiyang Zhang; > jbottom...@parallels.com; linux-s...@vger.kernel.org; > de...@linuxdriverproject.org; linux-ker...@vger.kernel.org > Subject: Re: [PATCH] scsi:storvsc enable reading from VPD pages on SPC-2 > > > "Long" == Long Li writes: > > >> Handle the issues or handle WRITE SAME(10/16)? > > Long> With this patch, the SCSI layer will be able to correctly send > Long> WRITE_SAME_16 to the Hyper-V host. It will not send WRITE_SAME_10: > Long> it has been disabled in the driver template. Do you want me to > Long> send another patch with these details? > > no_write_same prevents us from attempting to use WRITE SAME(10/16) to zero > block ranges. > > This is completely orthogonal to using the WRITE SAME(10/16) commands with > the UNMAP bit set to discard block ranges. The latter is controlled by the > logical > block provisioning heuristics and is not affected by no_write_same at all. > > -- > Martin K. PetersenOracle Linux Engineering ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] checkpatch giving bogus advice (was staging: goldfish: Fix minor coding style)
On Mon, 2014-12-15 at 14:59 +0300, Dan Carpenter wrote: > I prefer !foo because it is more common in the kernel and I think it's > easier to read but I don't feel strongly about this. Me too. But I do prefer consistency. fyi: for variants of: "if (!foo)" vs "if (foo == NULL)" a little cocci script shows a preference for "if (!foo)" of >5:1 in drivers/net/ (actual: 11557:2145) and a little less (>3:1) in net/ (actual: 10263:3045) $ cat pointer_style.cocci @@ type T; T *a; @@ * a == NULL @@ type T; T *a; @@ * a != NULL @@ type T; T *a; @@ * !a ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [HPDD-discuss] [PATCH] staging: lustre: lustre: obdclass: lprocfs_status.c: Fix for possible null pointer dereference
Strongly agreed that execution speed is not critical here. It's the update of
a proc variable, not a tight loop or critical section.
Normally I'd leave it alone, but since you're writing a patch anyway, I'd do
'tolower' myself. I dislike the stacked case statements on a single line like
that. (It's the only time I've seen them written that way. Perhaps it's
common and I've just missed it.)
Regards,
- Patrick
From: HPDD-discuss [hpdd-discuss-boun...@lists.01.org] on behalf of Joe Perches
[j...@perches.com]
Sent: Monday, December 15, 2014 5:53 PM
To: Rickard Strandqvist
Cc: de...@driverdev.osuosl.org; Fabian Frederick; Julia Lawall; James Simmons;
Greg Kroah-Hartman; linux-ker...@vger.kernel.org; Greg Donald;
hpdd-disc...@lists.01.org; Andriy Skulysh
Subject: Re: [HPDD-discuss] [PATCH] staging: lustre: lustre: obdclass:
lprocfs_status.c: Fix for possible null pointer dereference
On Mon, 2014-12-15 at 23:23 +0100, Rickard Strandqvist wrote:
> Hi Joe
Hello Rickard
> No, it does not look like end can be NULL then.
> Then remove the end != NULL instead?
> ...
> if (end != NULL && *end == '.') {
Up to you.
> However, I am hesitant to the tolower() I think double case is faster...?
I doubt code execution speed is paramount here.
Maybe see if the object code size is smaller one
way or the other.
___
HPDD-discuss mailing list
hpdd-disc...@lists.01.org
https://lists.01.org/mailman/listinfo/hpdd-discuss
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8192e: rejoin split quoted strings
Fix a checkpatch.pl warning regarding quoted string splits across lines.
While each join of these quoted strings results in a new checkpatch.pl
"lines over 80 characters" warning, the regained ability to grep for
these log strings in the codebase is, I would argue, well worth the
trade-off.
Signed-off-by: Jonathan Jin
---
drivers/staging/rtl8192e/rtllib_rx.c | 92
1 file changed, 29 insertions(+), 63 deletions(-)
diff --git a/drivers/staging/rtl8192e/rtllib_rx.c
b/drivers/staging/rtl8192e/rtllib_rx.c
index cf11b04..fdc349e 100644
--- a/drivers/staging/rtl8192e/rtllib_rx.c
+++ b/drivers/staging/rtl8192e/rtllib_rx.c
@@ -70,8 +70,7 @@ rtllib_frag_cache_find(struct rtllib_device *ieee, unsigned
int seq,
if (entry->skb != NULL &&
time_after(jiffies, entry->first_frag_time + 2 * HZ)) {
RTLLIB_DEBUG_FRAG(
- "expiring fragment cache entry "
- "seq=%u last_frag=%u\n",
+ "expiring fragment cache entry seq=%u
last_frag=%u\n",
entry->seq, entry->last_frag);
dev_kfree_skb_any(entry->skb);
entry->skb = NULL;
@@ -188,8 +187,7 @@ static int rtllib_frag_cache_invalidate(struct
rtllib_device *ieee,
if (entry == NULL) {
RTLLIB_DEBUG_FRAG(
- "could not invalidate fragment cache "
- "entry (seq=%u)\n", seq);
+ "could not invalidate fragment cache entry (seq=%u)\n",
seq);
return -1;
}
@@ -305,11 +303,9 @@ rtllib_rx_frame_decrypt(struct rtllib_device *ieee, struct
sk_buff *skb,
atomic_dec(&crypt->refcnt);
if (res < 0) {
RTLLIB_DEBUG_DROP(
- "decryption failed (SA= %pM"
- ") res=%d\n", hdr->addr2, res);
+ "decryption failed (SA= %pM) res=%d\n", hdr->addr2,
res);
if (res == -2)
- RTLLIB_DEBUG_DROP("Decryption failed ICV "
-"mismatch (key %d)\n",
+ RTLLIB_DEBUG_DROP("Decryption failed ICV mismatch (key
%d)\n",
skb->data[hdrlen + 3] >> 6);
ieee->ieee_stats.rx_discards_undecryptable++;
return -1;
@@ -345,8 +341,7 @@ rtllib_rx_frame_decrypt_msdu(struct rtllib_device *ieee,
struct sk_buff *skb,
res = crypt->ops->decrypt_msdu(skb, keyidx, hdrlen, crypt->priv);
atomic_dec(&crypt->refcnt);
if (res < 0) {
- printk(KERN_DEBUG "%s: MSDU decryption/MIC verification failed"
- " (SA= %pM keyidx=%d)\n",
+ printk(KERN_DEBUG "%s: MSDU decryption/MIC verification failed
(SA= %pM keyidx=%d)\n",
ieee->dev->name, hdr->addr2, keyidx);
return -1;
}
@@ -559,8 +554,7 @@ static void RxReorderIndicatePacket(struct rtllib_device
*ieee,
bool bMatchWinStart = false, bPktInBuf = false;
unsigned long flags;
- RTLLIB_DEBUG(RTLLIB_DL_REORDER, "%s(): Seq is %d, pTS->RxIndicateSeq"
-" is %d, WinSize is %d\n", __func__, SeqNum,
+ RTLLIB_DEBUG(RTLLIB_DL_REORDER, "%s(): Seq is %d, pTS->RxIndicateSeq is
%d, WinSize is %d\n", __func__, SeqNum,
pTS->RxIndicateSeq, WinSize);
spin_lock_irqsave(&(ieee->reorder_spinlock), flags);
@@ -600,8 +594,7 @@ static void RxReorderIndicatePacket(struct rtllib_device
*ieee,
pTS->RxIndicateSeq = SeqNum + 1 - WinSize;
else
pTS->RxIndicateSeq = 4095 - (WinSize - (SeqNum + 1)) +
1;
- RTLLIB_DEBUG(RTLLIB_DL_REORDER, "Window Shift! IndicateSeq: %d,"
-" NewSeq: %d\n", pTS->RxIndicateSeq, SeqNum);
+ RTLLIB_DEBUG(RTLLIB_DL_REORDER, "Window Shift! IndicateSeq: %d,
NewSeq: %d\n", pTS->RxIndicateSeq, SeqNum);
}
/*
@@ -617,8 +610,7 @@ static void RxReorderIndicatePacket(struct rtllib_device
*ieee,
*/
if (bMatchWinStart) {
/* Current packet is going to be indicated.*/
- RTLLIB_DEBUG(RTLLIB_DL_REORDER, "Packets indication!! "
- "IndicateSeq: %d, NewSeq: %d\n",
+ RTLLIB_DEBUG(RTLLIB_DL_REORDER, "Packets indication!!
IndicateSeq: %d, NewSeq: %d\n",
pTS->RxIndicateSeq, SeqNum);
ieee->prxbIndicateArray[0] = prxb;
index = 1;
@@ -636,9 +628,7 @@ static void RxReorderIndicatePacket(struct rtllib_device
*ieee,
if (!AddReorderEntry(pTS, pReorderEntry)) {
RTLLIB_DEBUG(RTLLIB_DL_REORDER,
-"%s(): Duplicate
Re: [PATCH] staging: lustre: lustre: obdclass: lprocfs_status.c: Fix for possible null pointer dereference
On Sun, Dec 14, 2014 at 4:52 PM, Rickard Strandqvist wrote: > units = 1; ... > /* Specified units override the multiplier */ > if (units) That doesn't make much sense. The conditional logic will always be executed. Maybe this is what's intended? /* Specified units override the multiplier */ - if (units) + if (units > 1) mult = mult < 0 ? -units : units; Chris ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] drivers: staging: lustre: Use mult if units not specified
Units can be passed to lprocfs_write_frac_u64_helper() via a suffix (e.g., "...K", "...M", etc.) tacked onto the value. A comment states that "specified units override the multiplier," though the multiplier is overridden regardless. Update the conditional logic so that it only applies when units are specified. Signed-off-by: Chris Rorvick --- drivers/staging/lustre/lustre/obdclass/lprocfs_status.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c index 61e04af..92ed0a0 100644 --- a/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c +++ b/drivers/staging/lustre/lustre/obdclass/lprocfs_status.c @@ -1910,7 +1910,7 @@ int lprocfs_write_frac_u64_helper(const char *buffer, unsigned long count, units <<= 10; } /* Specified units override the multiplier */ - if (units) + if (units > 1) mult = mult < 0 ? -units : units; frac *= mult; -- 2.1.0 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging:vt6655 Add extern variable in head file
From: Sun Wang Subject: [PATCH] staging:vt6655 Add extern variable in head file Extern variable must be declared in head file. Compiler can catch the inconsistency when variable changes. Signed-off-by: Sun Wang --- drivers/staging/vt6655/ioctl.c | 1 - drivers/staging/vt6655/ioctl.h | 5 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/staging/vt6655/ioctl.c b/drivers/staging/vt6655/ioctl.c index 970e80d..716fc89 100644 --- a/drivers/staging/vt6655/ioctl.c +++ b/drivers/staging/vt6655/ioctl.c @@ -31,7 +31,6 @@ */ #include "ioctl.h" -#include "iocmd.h" #include "mac.h" #include "card.h" #include "hostap.h" diff --git a/drivers/staging/vt6655/ioctl.h b/drivers/staging/vt6655/ioctl.h index 2dc5a57..42f6910 100644 --- a/drivers/staging/vt6655/ioctl.h +++ b/drivers/staging/vt6655/ioctl.h @@ -30,6 +30,11 @@ #define __IOCTL_H__ #include "device.h" +#include "iocmd.h" + +#ifdef WPA_SM_Transtatus +extern SWPAResult wpa_Result; +#endif int private_ioctl(struct vnt_private *, struct ifreq *rq); ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
