Question about the function,ni_stc_dma_channel_select_bitfield
Greetings All, I am wondering if in the function,ni_stc_dma_channel_select_bitfield the line: return 1 << channel; is guaranteed to be below the threshold that guarantees us to not overflow on a unsigned 32 integer due to bit wise shifting to the left. Thanks Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Question about the function,ni_stc_dma_channel_select_bitfield
On 2015-05-06 05:10 AM, Ian Abbott wrote: > On 06/05/15 01:22, nick wrote: >> Greetings All, >> I am wondering if in the function,ni_stc_dma_channel_select_bitfield the >> line: >> return 1 << channel; >> is guaranteed to be below the threshold that guarantees us to not overflow on >> a unsigned 32 integer due to bit wise shifting to the left. >> Thanks Nick >> > > if (channel < 4) > return 1 << channel; > > So, yes. > This should be commented in my option as this is not common knowledge unless you known the hardware specs really well. If I should send in a patch adding a comment here please let me known. Thanks, Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Question about the function,ni_stc_dma_channel_select_bitfield
On 2015-05-06 03:09 PM, Greg KH wrote: > On Wed, May 06, 2015 at 10:05:55AM -0400, nick wrote: >> On 2015-05-06 05:10 AM, Ian Abbott wrote: >>> On 06/05/15 01:22, nick wrote: >>>> Greetings All, >>>> I am wondering if in the function,ni_stc_dma_channel_select_bitfield the >>>> line: >>>> return 1 << channel; >>>> is guaranteed to be below the threshold that guarantees us to not overflow >>>> on >>>> a unsigned 32 integer due to bit wise shifting to the left. >>>> Thanks Nick >>>> >>> >>> if (channel < 4) >>> return 1 << channel; >>> >>> So, yes. >>> >> This should be commented in my option as this is not common knowledge >> unless you known the hardware specs really well. If I should send in >> a patch adding a comment here please let me known. > > Nick, this is one reason why you were banned from all vger.kernel.org > mailing lists. You are wasting people's time and refuse to listen to > others. > > I do give you credit for coming up with very strange questions that show > a complete lack of understanding of the C language, that is intertaining > at times, like this response, nice job with that. > > Please everyone, just add him to your killfile and ignore him, it makes > life much easier these days when he tries to go around the vger ban by > emailing developers directly as he did here. > > greg k-h > Greg Kh, Sorry about that :(, your absolutely right I need to listen more to the community. After thinking about it your right that was a terrible question in nature and I should have assumed based off the code that my assumptions were right. Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Build Warnings for the file, config-osm.c and i2o_config.c
Greetings All, I am getting the below build warnings for the file, config-osm.c and i2o_config whenever I do a complete clean allmodconfig build on my system. In file included from drivers/staging/i2o/config-osm.c:39:0: drivers/staging/i2o/i2o_config.c: In function ‘i2o_cfg_passthru’: drivers/staging/i2o/i2o_config.c:892:19: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] (p->virt, (void __user *)sg[i].addr_bus, ^ drivers/staging/i2o/i2o_config.c:952:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] ((void __user *)sg[j].addr_bus, sg_list[j].virt, ^ Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Patch Proof for Greg
Greetings Greg and others,
I am sending a two part patch fix to prove to Greg I have changed.
Below this message are the patches for a bug fix in the gma500 driver
code. Unfortunately I haven't gotten a maintainer acknowledge and
therefore it's not in the merge for the 4.1 release.
Nick
Patch 1
1 From 2d2ddb5d9a2c4fcbae45339d4f775fcde49f36e1 Mon Sep 17 00:00:00 2001
2 From: Nicholas Krause
3 Date: Wed, 13 May 2015 21:36:44 -0400
4 Subject: [PATCH 1/2] gma500:Add proper use of the variable ret for the
5 function, psb_mmu_inset_pfn_sequence
6
7 This adds proper use of the variable ret by returning it
8 at the end of the function, psb_mmu_inset_pfn_sequence for
9 indicating to callers when an error has occurred. Further
10 more remove the unneeded double setting of ret to the error
11 code, -ENOMEM after checking if a call to the function,
12 psb_mmu_pt_alloc_map_lock is successful.
13
14 Signed-off-by: Nicholas Krause
15 ---
16 drivers/gpu/drm/gma500/mmu.c | 7 +++
17 1 file changed, 3 insertions(+), 4 deletions(-)
18
19 diff --git a/drivers/gpu/drm/gma500/mmu.c b/drivers/gpu/drm/gma500/mmu.c
20 index 0eaf11c..d2c4bac 100644
21 --- a/drivers/gpu/drm/gma500/mmu.c
22 +++ b/drivers/gpu/drm/gma500/mmu.c
23 @@ -677,10 +677,9 @@ int psb_mmu_insert_pfn_sequence(struct psb_mmu_pd *pd,
uint32_t start_pfn,
24 do {
25 next = psb_pd_addr_end(addr, end);
26 pt = psb_mmu_pt_alloc_map_lock(pd, addr);
27 - if (!pt) {
28 - ret = -ENOMEM;
29 + if (!pt)
30 goto out;
31 - }
32 +
33 do {
34 pte = psb_mmu_mask_pte(start_pfn++, type);
35 psb_mmu_set_pte(pt, addr, pte);
36 @@ -700,7 +699,7 @@ out:
37 if (pd->hw_context != -1)
38 psb_mmu_flush(pd->driver);
39
40 - return 0;
41 + return ret;
42 }
43
44 int psb_mmu_insert_pages(struct psb_mmu_pd *pd, struct page **pages,
45 --
46 2.1.4
47
Patch 2
1 From f1802ff61ef69ff9ecaaeadb941d4ed725a0255a Mon Sep 17 00:00:00 2001
2 From: Nicholas Krause
3 Date: Fri, 22 May 2015 23:10:49 -0400
4 Subject: [PATCH 2/2] gma500: Add new error checking for the function,
psb_driver_load
5
6 This adds new error checking to the function, psb_driver_load
7 for when the function, psb_mmu_inset_pfn_sequence fails to grab
8 memory for the internal function call to psb_pd_addr_end. In
9 addition we now implement this by checking for a error code
10 return value from the internal call to the function,
11 psb_mmu_inset_pfn_sequence for the function, psb_driver_load.
12 If a error code is returned we must jump to a new label,
13 unlock_err in order to deallocate our usage count by
14 one for the usage of the semaphore, sem before unloading
15 the drm_device structure and returning a error code.
16
17 Signed-off-by: Nicholas Krause
18 ---
19 drivers/gpu/drm/gma500/psb_drv.c | 4
20 1 file changed, 4 insertions(+)
21
22 diff --git a/drivers/gpu/drm/gma500/psb_drv.c
b/drivers/gpu/drm/gma500/psb_drv.c
23 index 92e7e57..4cef2c9 100644
24 --- a/drivers/gpu/drm/gma500/psb_drv.c
25 +++ b/drivers/gpu/drm/gma500/psb_drv.c
26 @@ -343,6 +343,8 @@ static int psb_driver_load(struct drm_device *dev,
unsigned long flags)
27 dev_priv->stolen_base >>
PAGE_SHIFT,
28 pg->gatt_start,
29 pg->stolen_size >> PAGE_SHIFT, 0);
30 + if (ret)
31 + goto unlock_err;
32 up_read(&pg->sem);
33
34 psb_mmu_set_pd_context(psb_mmu_get_default_pd(dev_priv->mmu), 0);
35 @@ -405,6 +407,8 @@ static int psb_driver_load(struct drm_device *dev,
unsigned long flags)
36 #endif
37 /* Intel drm driver load is done, continue doing pvr load */
38 return 0;
39 +unlock_err:
40 + up_read(&pg->sem);
41 out_err:
42 psb_driver_unload(dev);
43 return ret;
44 --
45 2.1.4
46
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: PATCH[[vme/bridges/vme_ca91cx42.c:1382: Bad if test Bug Fix]
Here is the fixed patch. Having issues with using Thunderbird
so just used Evolution for now.
Nick
--- drivers/vme/bridges/vme_ca91cx42.h.orig 2014-06-11 22:50:29.339671939
-0400
+++ drivers/vme/bridges/vme_ca91cx42.h 2014-06-11 23:15:36.027685173 -0400
Fixes bug issues with wrong bus width in if statments in vme_ca91cx42.c
Signed-off-by: Nicholas Krause
@@ -526,7 +526,7 @@ static const int CA91CX42_LINT_LM[] = {
#define CA91CX42_VSI_CTL_SUPER_SUPR(1<<21)
#define CA91CX42_VSI_CTL_VAS_M (7<<16)
-#define CA91CX42_VSI_CTL_VAS_A16 0
+#define CA91CX42_VSI_CTL_VAS_A16 (3<<16)
#define CA91CX42_VSI_CTL_VAS_A24 (1<<16)
#define CA91CX42_VSI_CTL_VAS_A32 (1<<17)
#define CA91CX42_VSI_CTL_VAS_USER1 (3<<17)
@@ -549,7 +549,7 @@ static const int CA91CX42_LINT_LM[] = {
#define CA91CX42_LM_CTL_SUPR (1<<21)
#define CA91CX42_LM_CTL_NPRIV (1<<20)
#define CA91CX42_LM_CTL_AS_M (5<<16)
-#define CA91CX42_LM_CTL_AS_A16 0
+#define CA91CX42_LM_CTL_AS_A16 (3<<16)
#define CA91CX42_LM_CTL_AS_A24 (1<<16)
#define CA91CX42_LM_CTL_AS_A32 (1<<17)
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Patch[Fixes marco definition for if statements that use marco CA91CX42_LM_CTL_AS_A16 in header file, vme_ca91cx42.h]
Signed-off-by: Nicholas Krause
---
drivers/vme/bridges/vme_ca91cx42.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/vme/bridges/vme_ca91cx42.h
b/drivers/vme/bridges/vme_ca91cx42.h
index 02a7c79..57e942f 100644
--- a/drivers/vme/bridges/vme_ca91cx42.h
+++ b/drivers/vme/bridges/vme_ca91cx42.h
@@ -549,7 +549,7 @@ static const int CA91CX42_LINT_LM[] = { CA91CX42_LINT_LM0,
CA91CX42_LINT_LM1,
#define CA91CX42_LM_CTL_SUPR (1<<21)
#define CA91CX42_LM_CTL_NPRIV (1<<20)
#define CA91CX42_LM_CTL_AS_M (5<<16)
-#define CA91CX42_LM_CTL_AS_A16 0
+#define CA91CX42_LM_CTL_AS_A16 (3<<16)
#define CA91CX42_LM_CTL_AS_A24 (1<<16)
#define CA91CX42_LM_CTL_AS_A32 (1<<17)
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging wlan-ng: Add missing a blank line after declarations
On 14-09-15 10:26 PM, Nicholas Krause wrote:
> Fixing trivial checkpatch warnings about missing line after
> declarations.
>
> Signed-off-by: Nicholas Krause
> ---
> Tested by compilation only.
> drivers/staging/wlan-ng/hfa384x.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/staging/wlan-ng/hfa384x.h
> b/drivers/staging/wlan-ng/hfa384x.h
> index 1f2c78c..20d146b 100644
> --- a/drivers/staging/wlan-ng/hfa384x.h
> +++ b/drivers/staging/wlan-ng/hfa384x.h
> @@ -1376,6 +1376,7 @@ int hfa384x_drvr_setconfig(hfa384x_t *hw, u16 rid, void
> *buf, u16 len);
> static inline int hfa384x_drvr_getconfig16(hfa384x_t *hw, u16 rid, void *val)
> {
> int result = 0;
> +
> result = hfa384x_drvr_getconfig(hw, rid, val, sizeof(u16));
> if (result == 0)
> *((u16 *) val) = le16_to_cpu(*((u16 *) val));
> @@ -1385,6 +1386,7 @@ static inline int hfa384x_drvr_getconfig16(hfa384x_t
> *hw, u16 rid, void *val)
> static inline int hfa384x_drvr_setconfig16(hfa384x_t *hw, u16 rid, u16 val)
> {
> u16 value = cpu_to_le16(val);
> +
> return hfa384x_drvr_setconfig(hw, rid, &value, sizeof(value));
> }
> @@ -1402,6 +1404,7 @@ static inline int
> hfa384x_drvr_setconfig16_async(hfa384x_t *hw, u16 rid, u16 val)
> {
> u16 value = cpu_to_le16(val);
> +
> return hfa384x_drvr_setconfig_async(hw, rid, &value, sizeof(value),
> NULL, NULL);
> }
> 1.9.1
>
This version is the correct version of my patch. The other one was not check
patched and please forget about it.
Nick
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused
On Wed, Sep 26, 2018 at 10:55 AM Nick Desaulniers
wrote:
>
> On Wed, Sep 26, 2018 at 12:41 AM Nathan Chancellor
> wrote:
> >
> > On Wed, Sep 26, 2018 at 09:13:59AM +0200, Greg Kroah-Hartman wrote:
> > > On Wed, Sep 26, 2018 at 12:02:09AM -0700, Nathan Chancellor wrote:
> > > > Clang emits the following warning:
> > > >
> > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning: variable
> > > > 'acpi_ids' is not needed and will not be emitted
> > > > [-Wunneeded-internal-declaration]
> > > > static const struct acpi_device_id acpi_ids[] = {
> > > >^
> > > > 1 warning generated.
> > > >
> > > > Mark the declaration as maybe unused like a few other instances of this
> > > > construct in the kernel.
> > > >
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/169
> > > > Signed-off-by: Nathan Chancellor
> > > > ---
> > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > index 6d02904de63f..3285bf36291b 100644
> > > > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] =
> > > > { SDIO_DEVICE(0x024c, 0xb723), },
> > > > { /* end: all zeroes */ },
> > > > };
> > > > -static const struct acpi_device_id acpi_ids[] = {
> > > > +static const struct acpi_device_id acpi_ids[] __maybe_unused = {
> > >
> > > But it is used. No "maybe" at all here. The MODULE_DEVICE_TABLE()
> > > macro does a functional thing. Why is gcc not reporting an issue with
> > > this and clang is?
>
> Looks like GCC and Clang handle __attribute__((alias)) differently
> when optimizations are enabled. Clang has
> -Wunneeded-internal-declaration (GCC doesn't have that specific flag,
> which is why GCC doesn't report, but its behavior is also different),
> as part of -Wall, which warns that it thinks the static var is dead,
> then removes it from -O1 and up.
>
> https://godbolt.org/z/Dpxnbp
>
> I consider this a bug in Clang: https://bugs.llvm.org/show_bug.cgi?id=39088
>
> As Nathan notes in this comment in V1:
> https://lore.kernel.org/lkml/20180926064437.GA29417@flashbox/ having
> additional references to the static array is enough to keep it around.
> When there are no other references, then it should not be marked
> static, in order to still be emitted.
>
> We can work around this by removing `static` from struct definitions
> that no other references than being passed to the MODULE_DEVICE_TABLE
> macro. GCC and Clang will then both emit references to both
> identifiers.
Sorry, after thinking more about this and discussing more with a
coworker, I think Clang is doing the right thing, and that `static`
should be removed from declarations passed to MODULE_DEVICE_TABLE
without other references. Clang's Dead Code Elimination (DCE) here
seems to be more aggressive, but it still looks correct to me.
int foo [] = { 42, 0xDEAD }; // extern
extern typeof(foo) bar __attribute__((unused, alias("foo")));
GCC and Clang at -O2 both produce references to foo and bar.
Adding `static` to foo, then GCC produces both references, while Clang
moves the data to bar and removes foo. This is safe because foo has
no other references, and bar is what file2alias.c cares about in this
case.
--
Thanks,
~Nick Desaulniers
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as maybe unused
On Wed, Sep 26, 2018 at 11:39 AM Greg KH wrote:
>
> On Wed, Sep 26, 2018 at 11:28:46AM -0700, Nick Desaulniers wrote:
> > On Wed, Sep 26, 2018 at 10:55 AM Nick Desaulniers
> > wrote:
> > >
> > > On Wed, Sep 26, 2018 at 12:41 AM Nathan Chancellor
> > > wrote:
> > > >
> > > > On Wed, Sep 26, 2018 at 09:13:59AM +0200, Greg Kroah-Hartman wrote:
> > > > > On Wed, Sep 26, 2018 at 12:02:09AM -0700, Nathan Chancellor wrote:
> > > > > > Clang emits the following warning:
> > > > > >
> > > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c:25:36: warning:
> > > > > > variable
> > > > > > 'acpi_ids' is not needed and will not be emitted
> > > > > > [-Wunneeded-internal-declaration]
> > > > > > static const struct acpi_device_id acpi_ids[] = {
> > > > > >^
> > > > > > 1 warning generated.
> > > > > >
> > > > > > Mark the declaration as maybe unused like a few other instances of
> > > > > > this
> > > > > > construct in the kernel.
> > > > > >
> > > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/169
> > > > > > Signed-off-by: Nathan Chancellor
> > > > > > ---
> > > > > > drivers/staging/rtl8723bs/os_dep/sdio_intf.c | 2 +-
> > > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > > > b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > > > index 6d02904de63f..3285bf36291b 100644
> > > > > > --- a/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > > > +++ b/drivers/staging/rtl8723bs/os_dep/sdio_intf.c
> > > > > > @@ -22,7 +22,7 @@ static const struct sdio_device_id sdio_ids[] =
> > > > > > { SDIO_DEVICE(0x024c, 0xb723), },
> > > > > > { /* end: all zeroes */ },
> > > > > > };
> > > > > > -static const struct acpi_device_id acpi_ids[] = {
> > > > > > +static const struct acpi_device_id acpi_ids[] __maybe_unused = {
> > > > >
> > > > > But it is used. No "maybe" at all here. The MODULE_DEVICE_TABLE()
> > > > > macro does a functional thing. Why is gcc not reporting an issue with
> > > > > this and clang is?
> > >
> > > Looks like GCC and Clang handle __attribute__((alias)) differently
> > > when optimizations are enabled. Clang has
> > > -Wunneeded-internal-declaration (GCC doesn't have that specific flag,
> > > which is why GCC doesn't report, but its behavior is also different),
> > > as part of -Wall, which warns that it thinks the static var is dead,
> > > then removes it from -O1 and up.
> > >
> > > https://godbolt.org/z/Dpxnbp
> > >
> > > I consider this a bug in Clang:
> > > https://bugs.llvm.org/show_bug.cgi?id=39088
> > >
> > > As Nathan notes in this comment in V1:
> > > https://lore.kernel.org/lkml/20180926064437.GA29417@flashbox/ having
> > > additional references to the static array is enough to keep it around.
> > > When there are no other references, then it should not be marked
> > > static, in order to still be emitted.
> > >
> > > We can work around this by removing `static` from struct definitions
> > > that no other references than being passed to the MODULE_DEVICE_TABLE
> > > macro. GCC and Clang will then both emit references to both
> > > identifiers.
> >
> > Sorry, after thinking more about this and discussing more with a
> > coworker, I think Clang is doing the right thing, and that `static`
> > should be removed from declarations passed to MODULE_DEVICE_TABLE
> > without other references. Clang's Dead Code Elimination (DCE) here
> > seems to be more aggressive, but it still looks correct to me.
> >
> > int foo [] = { 42, 0xDEAD }; // extern
> > extern typeof(foo) bar __attribute__((unused, alias("foo")));
> >
> > GCC and Clang at -O2 both produce references to foo and bar.
> >
> > Adding `static` to foo, then GCC produces both references, while Clang
> > moves the data to bar and removes foo. This is safe because foo has
> > no other references, and bar is what file2alias.c cares a
Re: [PATCH 000/141] Fix fall-through warnings for Clang
On Sun, Nov 22, 2020 at 8:17 AM Kees Cook wrote:
>
> On Fri, Nov 20, 2020 at 11:51:42AM -0800, Jakub Kicinski wrote:
> > If none of the 140 patches here fix a real bug, and there is no change
> > to machine code then it sounds to me like a W=2 kind of a warning.
>
> FWIW, this series has found at least one bug so far:
> https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/
So looks like the bulk of these are:
switch (x) {
case 0:
++x;
default:
break;
}
I have a patch that fixes those up for clang:
https://reviews.llvm.org/D91895
There's 3 other cases that don't quite match between GCC and Clang I
observe in the kernel:
switch (x) {
case 0:
++x;
default:
goto y;
}
y:;
switch (x) {
case 0:
++x;
default:
return;
}
switch (x) {
case 0:
++x;
default:
;
}
Based on your link, and Nathan's comment on my patch, maybe Clang
should continue to warn for the above (at least the `default: return;`
case) and GCC should change? While the last case looks harmless,
there were only 1 or 2 across the tree in my limited configuration
testing; I really think we should just add `break`s for those.
--
Thanks,
~Nick Desaulniers
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8723bs: Mark ACPI table declaration as used
On Wed, Jan 9, 2019 at 11:01 PM Greg KH wrote: > > On Wed, Jan 09, 2019 at 04:19:30PM -0800, Nick Desaulniers wrote: > > Digging up an old thread to point out I was wrong: > > > > On Thu, Sep 27, 2018 at 1:08 PM Nick Desaulniers > > wrote: > > > > This is not a million-little-commits problem; more like 2 drivers in > > > > the whole tree have this problem. > > > > > > > > Note that this change that Nathan has DOES NOT need to applied to > > > > every driver that uses MODULE_DEVICE_TABLE. Only when the struct has > > > > no other references other than JUST being passed to > > > > MODULE_DEVICE_TABLE() is this a problem. For an allyesconfig, I only > > > > see 2 instances of this case in the whole tree: > > > > > > > > https://github.com/ClangBuiltLinux/linux/issues/169 <- this patch is > > > > addressing > > > > https://github.com/ClangBuiltLinux/linux/issues/178 <- waiting on the > > > > results of this patch > > > > > > > > If you're ok with this change to two drivers, then: > > > > Reviewed-by: Nick Desaulniers > > > > I just landed a fix for this in Clang > > (https://reviews.llvm.org/rL350776) which will ship soon in Clang 8. > > Sedat helped test this patch and reported that it removed 41 > > false-negative warnings > > https://github.com/ClangBuiltLinux/linux/issues/232#issuecomment-440006399 > > Great, thanks for letting me know. Can I revert this patch? Go for it. I think my mistake was that we had 2 instances of this listed in our bug tracker, but an allyesconfig had many many more instances than I was aware of. -- Thanks, ~Nick Desaulniers ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: kpc2000: Use memset to initialize resources
On Wed, Apr 24, 2019 at 11:58 AM Nathan Chancellor
wrote:
>
> Clang warns:
>
> drivers/staging/kpc2000/kpc2000/cell_probe.c:96:38: warning: suggest
> braces around initialization of subobject [-Wmissing-braces]
> struct resource resources[2] = {0};
> ^
> {}
> drivers/staging/kpc2000/kpc2000/cell_probe.c:314:38: warning: suggest
> braces around initialization of subobject [-Wmissing-braces]
> struct resource resources[2] = {0};
> ^
> {}
> 2 warnings generated.
>
> One way to fix these warnings is to add additional braces like Clang
> suggests; however, there has been a bit of push back from some
> maintainers, who just prefer memset as it is unambiguous, doesn't
> depend on a particular compiler version, and properly initializes all
> subobjects [1][2]. Do that here so there are no more warnings.
>
> [1]:
> https://lore.kernel.org/lkml/022e41c0-8465-dc7a-a45c-64187ecd9...@amd.com/
> [2]:
> https://lore.kernel.org/lkml/20181128.215241.702406654469517539.da...@davemloft.net/
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/455
> Signed-off-by: Nathan Chancellor
> ---
> drivers/staging/kpc2000/kpc2000/cell_probe.c | 12
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/kpc2000/kpc2000/cell_probe.c
> b/drivers/staging/kpc2000/kpc2000/cell_probe.c
> index ad2cc0a3bfa1..13f544f3c0b9 100644
> --- a/drivers/staging/kpc2000/kpc2000/cell_probe.c
> +++ b/drivers/staging/kpc2000/kpc2000/cell_probe.c
> @@ -93,8 +93,8 @@ void parse_core_table_entry(struct core_table_entry *cte,
> const u64 read_val, co
> int probe_core_basic(unsigned int core_num, struct kp2000_device *pcard,
> char *name, const struct core_table_entry cte)
> {
> struct mfd_cell cell = {0};
> -struct resource resources[2] = {0};
> -
> +struct resource resources[2];
> +
> struct kpc_core_device_platdata core_pdata = {
> .card_id = pcard->card_id,
> .build_version = pcard->build_version,
> @@ -112,6 +112,8 @@ int probe_core_basic(unsigned int core_num, struct
> kp2000_device *pcard, char *
> cell.id = core_num;
> cell.num_resources = 2;
>
> +memset(&resources, 0, sizeof(resources));
> +
> resources[0].start = cte.offset;
> resources[0].end = cte.offset + (cte.length - 1);
> resources[0].flags = IORESOURCE_MEM;
> @@ -311,8 +313,8 @@ int probe_core_uio(unsigned int core_num, struct
> kp2000_device *pcard, char *na
> static int create_dma_engine_core(struct kp2000_device *pcard, size_t
> engine_regs_offset, int engine_num, int irq_num)
> {
> struct mfd_cell cell = {0};
> -struct resource resources[2] = {0};
> -
> +struct resource resources[2];
> +
> dev_dbg(&pcard->pdev->dev, "create_dma_core(pcard = [%p],
> engine_regs_offset = %zx, engine_num = %d)\n", pcard, engine_regs_offset,
> engine_num);
>
> cell.platform_data = NULL;
> @@ -321,6 +323,8 @@ static int create_dma_engine_core(struct kp2000_device
> *pcard, size_t engine_re
> cell.name = KP_DRIVER_NAME_DMA_CONTROLLER;
> cell.num_resources = 2;
>
> +memset(&resources, 0, sizeof(resources));
> +
Bonus points for clearing up the leading whitespace. Thanks for the
patch. I'm beginning to think aggregate initializers are the devil.
Reviewed-by: Nick Desaulniers
> resources[0].start = engine_regs_offset;
> resources[0].end = engine_regs_offset + (KPC_DMA_ENGINE_SIZE - 1);
> resources[0].flags = IORESOURCE_MEM;
> --
> 2.21.0
>
--
Thanks,
~Nick Desaulniers
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192u: Remove an unnecessary NULL check
On Tue, May 21, 2019 at 10:42 AM Nathan Chancellor
wrote:
>
> Clang warns:
>
> drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:2663:47: warning:
> address of array 'param->u.wpa_ie.data' will always evaluate to 'true'
> [-Wpointer-bool-conversion]
> (param->u.wpa_ie.len && !param->u.wpa_ie.data))
> ~^~~~
>
> This was exposed by commit deabe03523a7 ("Staging: rtl8192u: ieee80211:
> Use !x in place of NULL comparisons") because we disable the warning
> that would have pointed out the comparison against NULL is also false:
>
> drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c:2663:46: warning:
> comparison of array 'param->u.wpa_ie.data' equal to a null pointer is
> always false [-Wtautological-pointer-compare]
> (param->u.wpa_ie.len && param->u.wpa_ie.data == NULL))
> ^~~~
>
> Remove it so clang no longer warns.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/487
> Signed-off-by: Nathan Chancellor
> ---
> drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> index f38f9d8b78bb..e0da0900a4f7 100644
> --- a/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> +++ b/drivers/staging/rtl8192u/ieee80211/ieee80211_softmac.c
> @@ -2659,8 +2659,7 @@ static int ieee80211_wpa_set_wpa_ie(struct
> ieee80211_device *ieee,
> {
> u8 *buf;
>
> - if (param->u.wpa_ie.len > MAX_WPA_IE_LEN ||
> - (param->u.wpa_ie.len && !param->u.wpa_ie.data))
Right so, the types in this expression:
param: struct ieee_param*
param->u: *anonymous union*
param->u.wpa_ie: *anonymous struct*
param->u.wpa_ie.len: u32
param->u.wpa_ie.data: u8 [0]
as defined in drivers/staging/rtl8192u/ieee80211/ieee80211.h#L295
https://github.com/ClangBuiltLinux/linux/blob/9c7db5004280767566e91a33445bf93aa479ef02/drivers/staging/rtl8192u/ieee80211/ieee80211.h#L295-L322
so this is a tricky case, because in general array members can never
themselves be NULL, and usually I trust -Wpointer-bool-conversion, but
this is a special case because of the flexible array member:
https://en.wikipedia.org/wiki/Flexible_array_member. (It seems that
having the 0 in the length explicitly was pre-c99 GNU extension:
https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html). I wonder if
-Wtautological-pointer-compare applies to Flexible Array Members or
not (Richard, do you know)? In general, you'd be setting
param->u.wpa_ie to the return value of a dynamic memory allocation,
not param->u.wpa_ie.data, so this check is fishy to me.
> + if (param->u.wpa_ie.len > MAX_WPA_IE_LEN)
> return -EINVAL;
>
> if (param->u.wpa_ie.len) {
> --
> 2.22.0.rc1
>
--
Thanks,
~Nick Desaulniers
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: comedi: comedidev.h Fixed whitespace coding style warnings
Fixed coding style warnings in comedidev.h which had an extra space
after the function pointer name.
Signed-off-by: Nick Davies
---
drivers/staging/comedi/comedidev.h | 48 +++---
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/staging/comedi/comedidev.h
b/drivers/staging/comedi/comedidev.h
index f82bd42..6ad4387 100644
--- a/drivers/staging/comedi/comedidev.h
+++ b/drivers/staging/comedi/comedidev.h
@@ -61,31 +61,31 @@ struct comedi_subdevice {
unsigned int *chanlist; /* driver-owned chanlist (not used) */
- int (*insn_read) (struct comedi_device *, struct comedi_subdevice *,
+ int (*insn_read)(struct comedi_device *, struct comedi_subdevice *,
+struct comedi_insn *, unsigned int *);
+ int (*insn_write)(struct comedi_device *, struct comedi_subdevice *,
struct comedi_insn *, unsigned int *);
- int (*insn_write) (struct comedi_device *, struct comedi_subdevice *,
+ int (*insn_bits)(struct comedi_device *, struct comedi_subdevice *,
+struct comedi_insn *, unsigned int *);
+ int (*insn_config)(struct comedi_device *, struct comedi_subdevice *,
struct comedi_insn *, unsigned int *);
- int (*insn_bits) (struct comedi_device *, struct comedi_subdevice *,
- struct comedi_insn *, unsigned int *);
- int (*insn_config) (struct comedi_device *, struct comedi_subdevice *,
- struct comedi_insn *, unsigned int *);
-
- int (*do_cmd) (struct comedi_device *, struct comedi_subdevice *);
- int (*do_cmdtest) (struct comedi_device *, struct comedi_subdevice *,
- struct comedi_cmd *);
- int (*poll) (struct comedi_device *, struct comedi_subdevice *);
- int (*cancel) (struct comedi_device *, struct comedi_subdevice *);
+
+ int (*do_cmd)(struct comedi_device *, struct comedi_subdevice *);
+ int (*do_cmdtest)(struct comedi_device *, struct comedi_subdevice *,
+ struct comedi_cmd *);
+ int (*poll)(struct comedi_device *, struct comedi_subdevice *);
+ int (*cancel)(struct comedi_device *, struct comedi_subdevice *);
/* int (*do_lock)(struct comedi_device *, struct comedi_subdevice *); */
/* int (*do_unlock)(struct comedi_device *, \
struct comedi_subdevice *); */
/* called when the buffer changes */
- int (*buf_change) (struct comedi_device *dev,
- struct comedi_subdevice *s, unsigned long new_size);
+ int (*buf_change)(struct comedi_device *dev,
+ struct comedi_subdevice *s, unsigned long new_size);
- void (*munge) (struct comedi_device *dev, struct comedi_subdevice *s,
- void *data, unsigned int num_bytes,
- unsigned int start_chan_index);
+ void (*munge)(struct comedi_device *dev, struct comedi_subdevice *s,
+ void *data, unsigned int num_bytes,
+ unsigned int start_chan_index);
enum dma_data_direction async_dma_dir;
unsigned int state;
@@ -146,8 +146,8 @@ struct comedi_async {
unsigned int cb_mask;
- int (*inttrig) (struct comedi_device *dev, struct comedi_subdevice *s,
- unsigned int x);
+ int (*inttrig)(struct comedi_device *dev, struct comedi_subdevice *s,
+ unsigned int x);
};
struct comedi_driver {
@@ -155,9 +155,9 @@ struct comedi_driver {
const char *driver_name;
struct module *module;
- int (*attach) (struct comedi_device *, struct comedi_devconfig *);
- void (*detach) (struct comedi_device *);
- int (*auto_attach) (struct comedi_device *, unsigned long);
+ int (*attach)(struct comedi_device *, struct comedi_devconfig *);
+ void (*detach)(struct comedi_device *);
+ int (*auto_attach)(struct comedi_device *, unsigned long);
/* number of elements in board_name and board_id arrays */
unsigned int num_names;
@@ -202,8 +202,8 @@ struct comedi_device {
struct fasync_struct *async_queue;
- int (*open) (struct comedi_device *dev);
- void (*close) (struct comedi_device *dev);
+ int (*open)(struct comedi_device *dev);
+ void (*close)(struct comedi_device *dev);
};
static inline const void *comedi_board(const struct comedi_device *dev)
--
1.8.5.3
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] Staging: comedi: ni_tio Fixed whitespace coding style warnings
Fixed coding style warnings in ni_tio.h which had an extra space
after the function pointer name.
Signed-off-by: Nick Davies
---
drivers/staging/comedi/drivers/ni_tio.h | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/comedi/drivers/ni_tio.h
b/drivers/staging/comedi/drivers/ni_tio.h
index 68378da..1056bf0 100644
--- a/drivers/staging/comedi/drivers/ni_tio.h
+++ b/drivers/staging/comedi/drivers/ni_tio.h
@@ -115,10 +115,10 @@ struct ni_gpct {
struct ni_gpct_device {
struct comedi_device *dev;
- void (*write_register) (struct ni_gpct *counter, unsigned bits,
- enum ni_gpct_register reg);
- unsigned (*read_register) (struct ni_gpct *counter,
- enum ni_gpct_register reg);
+ void (*write_register)(struct ni_gpct *counter, unsigned bits,
+ enum ni_gpct_register reg);
+ unsigned (*read_register)(struct ni_gpct *counter,
+ enum ni_gpct_register reg);
enum ni_gpct_variant variant;
struct ni_gpct *counters;
unsigned num_counters;
--
1.9.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] Drivers: hv: vmbus: Add support for VMBus panic notifier handler
Hyper-V allows a guest to notify the Hyper-V host that a panic
condition occured. This notification can include up to five 64
bit values. These 64 bit values are written into crash MSRs.
Once the data has been written into the crash MSRs, the host is
then notified by writing into a Crash Control MSR. On the Hyper-V
host, the panic notification data is captured in the Windows Event
log as a 18590 event.
Crash MSRs are defined in appendix H of the Hypervisor Top Level
Functional Specification. At the time of this patch, v4.0 is the
current functional spec. The URL for the v4.0 document is:
http://download.microsoft.com/download/A/B/4/AB43A34E-BDD0-4FA6-BDEF-79EEF16E880B/Hypervisor
Top Level Functional Specification v4.0.docx
Signed-off-by: Nick Meier
---
drivers/hv/hyperv_vmbus.h | 11 +++
drivers/hv/vmbus_drv.c| 36
2 files changed, 47 insertions(+)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 44b1c94..e54825c 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -49,6 +49,17 @@ enum hv_cpuid_function {
HVCPUID_IMPLEMENTATION_LIMITS = 0x4005,
};
+#define HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE 0x400
+
+#define HV_X64_MSR_CRASH_P0 0x4100
+#define HV_X64_MSR_CRASH_P1 0x4101
+#define HV_X64_MSR_CRASH_P2 0x4102
+#define HV_X64_MSR_CRASH_P3 0x4103
+#define HV_X64_MSR_CRASH_P4 0x4104
+#define HV_X64_MSR_CRASH_CTL 0x4105
+
+#define HV_CRASH_CTL_CRASH_NOTIFY 0x8000
+
/* Define version of the synthetic interrupt controller. */
#define HV_SYNIC_VERSION (1)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index f518b8d7..5e7ba5f 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -36,6 +36,8 @@
#include
#include
#include
+#include
+#include
#include "hyperv_vmbus.h"
static struct acpi_device *hv_acpi_dev;
@@ -44,6 +46,31 @@ static struct tasklet_struct msg_dpc;
static struct completion probe_event;
static int irq;
+
+int hyperv_panic_event(struct notifier_block *nb,
+ unsigned long event, void *ptr)
+{
+ struct pt_regs *regs;
+
+ regs = current_pt_regs();
+
+ wrmsrl(HV_X64_MSR_CRASH_P0, regs->ip);
+ wrmsrl(HV_X64_MSR_CRASH_P1, regs->ax);
+ wrmsrl(HV_X64_MSR_CRASH_P2, regs->bx);
+ wrmsrl(HV_X64_MSR_CRASH_P3, regs->cx);
+ wrmsrl(HV_X64_MSR_CRASH_P4, regs->dx);
+
+ /*
+* Let Hyper-V know there is crash data available
+*/
+ wrmsrl(HV_X64_MSR_CRASH_CTL, HV_CRASH_CTL_CRASH_NOTIFY);
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block hyperv_panic_block = {
+ .notifier_call = hyperv_panic_event,
+};
+
struct resource hyperv_mmio = {
.name = "hyperv mmio",
.flags = IORESOURCE_MEM,
@@ -744,6 +771,15 @@ static int vmbus_bus_init(int irq)
if (ret)
goto err_alloc;
+ /*
+* Only register if the crash MSRs are available
+*/
+ if (ms_hyperv.features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+ atomic_notifier_chain_register(&panic_notifier_list,
+ &hyperv_panic_block);
+ pr_info("Panic notifier handler registered\n");
+ }
+
vmbus_request_offers();
return 0;
--
2.1.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [char-misc:char-misc-testing 25/45] drivers/hv/vmbus_drv.c:67:9: sparse: constant 0x8000000000000000 is so big it is unsigned long
I will correct this constant definition and resubmit.
-Nick Meier
-Original Message-
From: kbuild test robot [mailto:fengguang...@intel.com]
Sent: Sunday, March 1, 2015 8:49 PM
To: Nick Meier
Cc: kbuild-...@01.org; Greg Kroah-Hartman; KY Srinivasan; Haiyang Zhang;
de...@linuxdriverproject.org; linux-ker...@vger.kernel.org
Subject: [char-misc:char-misc-testing 25/45] drivers/hv/vmbus_drv.c:67:9:
sparse: constant 0x8000 is so big it is unsigned long
tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
char-misc-testing
head: b3de8e3719e582f3182bb504295e4a8e43c8c96f
commit: 96c1d0581d00f7abe033350edb021a9d947d8d81 [25/45] Drivers: hv: vmbus:
Add support for VMBus panic notifier handler
reproduce:
# apt-get install sparse
git checkout 96c1d0581d00f7abe033350edb021a9d947d8d81
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> drivers/hv/vmbus_drv.c:67:9: sparse: constant 0x8000 is so big
>> it is unsigned long
>> drivers/hv/vmbus_drv.c:67:9: sparse: constant 0x8000 is so big
>> it is unsigned long
>> drivers/hv/vmbus_drv.c:51:5: sparse: symbol 'hyperv_panic_event' was not
>> declared. Should it be static?
>> drivers/hv/vmbus_drv.c:67:9: sparse: cast truncates bits from constant value
>> (8000 becomes 0)
Please review and possibly fold the followup patch.
vim +67 drivers/hv/vmbus_drv.c
45
46 static struct tasklet_struct msg_dpc;
47 static struct completion probe_event;
48 static int irq;
49
50
> 51 int hyperv_panic_event(struct notifier_block *nb,
52 unsigned long event, void *ptr)
53 {
54 struct pt_regs *regs;
55
56 regs = current_pt_regs();
57
58 wrmsrl(HV_X64_MSR_CRASH_P0, regs->ip);
59 wrmsrl(HV_X64_MSR_CRASH_P1, regs->ax);
60 wrmsrl(HV_X64_MSR_CRASH_P2, regs->bx);
61 wrmsrl(HV_X64_MSR_CRASH_P3, regs->cx);
62 wrmsrl(HV_X64_MSR_CRASH_P4, regs->dx);
63
64 /*
65 * Let Hyper-V know there is crash data available
66 */
> 67 wrmsrl(HV_X64_MSR_CRASH_CTL, HV_CRASH_CTL_CRASH_NOTIFY);
68 return NOTIFY_DONE;
69 }
70
---
0-DAY kernel test infrastructureOpen Source Technology Center
http://lists.01.org/mailman/listinfo/kbuild Intel Corporation
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
RE: [PATCH char-misc] mei: bus: () can be static
Yes - the function should have been static. This change correctly corrects the
oversight.
thanks,
-Nick
-Original Message-
From: kbuild test robot [mailto:fengguang...@intel.com]
Sent: Sunday, March 1, 2015 8:49 PM
To: Tomas Winkler
Cc: kbuild-...@01.org; Greg Kroah-Hartman; Nick Meier; KY Srinivasan; Haiyang
Zhang; de...@linuxdriverproject.org; linux-ker...@vger.kernel.org
Subject: [PATCH char-misc] mei: bus: () can be static
drivers/hv/vmbus_drv.c:51:5: sparse: symbol 'hyperv_panic_event' was not
declared. Should it be static?
drivers/hv/vmbus_drv.c:51:5: sparse: symbol 'hyperv_panic_event' was not
declared. Should it be static?
drivers/hv/vmbus_drv.c:51:5: sparse: symbol 'hyperv_panic_event' was not
declared. Should it be static?
drivers/hv/vmbus_drv.c:51:5: sparse: symbol 'hyperv_panic_event' was not
declared. Should it be static?
Signed-off-by: Fengguang Wu
---
vmbus_drv.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index
526fa8b..8313e25 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -48,7 +48,7 @@ static struct completion probe_event; static int irq;
-int hyperv_panic_event(struct notifier_block *nb,
+static int hyperv_panic_event(struct notifier_block *nb,
unsigned long event, void *ptr)
{
struct pt_regs *regs;
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/1] Correcting truncation error for constant HV_CRASH_CTL_CRASH_NOTIFY
HV_CRASH_CTL_CRASH_NOTIFY is a 64 bit number. Depending on the usage context,
the value may be truncated. This patch is in response from the following
email from Intel:
[char-misc:char-misc-testing 25/45] drivers/hv/vmbus_drv.c:67:9: sparse:
constant 0x8000 is so big it is unsigned long
tree: git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
char-misc-testing
head: b3de8e3719e582f3182bb504295e4a8e43c8c96f
commit: 96c1d0581d00f7abe033350edb021a9d947d8d81 [25/45] Drivers: hv:
vmbus: Add support for VMBus panic notifier handler
reproduce:
# apt-get install sparse
git checkout 96c1d0581d00f7abe033350edb021a9d947d8d81
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
drivers/hv/vmbus_drv.c:67:9: sparse: constant 0x8000 is so big
it is unsigned long
...
Signed-off-by: Nick Meier
---
drivers/hv/hyperv_vmbus.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index e54825c..d152967 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -58,7 +58,7 @@ enum hv_cpuid_function {
#define HV_X64_MSR_CRASH_P4 0x4104
#define HV_X64_MSR_CRASH_CTL 0x4105
-#define HV_CRASH_CTL_CRASH_NOTIFY 0x8000
+#define HV_CRASH_CTL_CRASH_NOTIFY (1ULL << 63)
/* Define version of the synthetic interrupt controller. */
#define HV_SYNIC_VERSION (1)
--
2.1.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] octeon-usb:Fix coding style issue with space between function name and opening bracket
On Tue, Mar 24, 2015 at 9:20 AM, Joe Perches wrote: > On Tue, 2015-03-24 at 16:00 +0300, Dan Carpenter wrote: >> On Tue, Mar 24, 2015 at 08:08:16AM -0400, Nicholas Krause wrote: >> > Just one more question, is that patch going to be merged or should I >> > resubmit >> > it as a series of one patch? >> Don't resubmit. Wait for Greg to mail you. It can take a several weeks >> because he is busy. > > What Dan said is certainly true, but if after 4 or > more weeks with no reply, I think it's fine to > resubmit or send a polite "ping" requesting status. > > There really no other option as no other feedback > mechanism exists and things can and do get lost in > high-volume process. > > That's fine, I will wait then for a reply from Greg. Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ks7010: Add spaces around '*'
Added spaces around a '*' in ks7010_sdio.c. Issue found by checkpatch.
Signed-off-by: Nick Rosbrook
---
drivers/staging/ks7010/ks7010_sdio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c
b/drivers/staging/ks7010/ks7010_sdio.c
index b02980d..55ece81 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -719,7 +719,7 @@ static int ks7010_sdio_update_index(struct ks_wlan_private
*priv, u32 index)
return rc;
}
-#define ROM_BUFF_SIZE (64*1024)
+#define ROM_BUFF_SIZE (64 * 1024)
static int ks7010_sdio_data_compare(struct ks_wlan_private *priv, u32 address,
unsigned char *data, unsigned int size)
{
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ks7010: Corrected a spelling mistake
This patch corrects the spelling of 'initialize' in ks7010_sdio.c. The issue was found by checkpatch. Signed-off-by: Nick Rosbrook --- drivers/staging/ks7010/ks7010_sdio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c index b02980d..039efce 100644 --- a/drivers/staging/ks7010/ks7010_sdio.c +++ b/drivers/staging/ks7010/ks7010_sdio.c @@ -956,7 +956,7 @@ static int ks7010_sdio_probe(struct sdio_func *func, priv = NULL; netdev = NULL; - /* initilize ks_sdio_card */ + /* initialize ks_sdio_card */ card = kzalloc(sizeof(*card), GFP_KERNEL); if (!card) return -ENOMEM; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: ks7010: ks7010_sdio.c: Fixing multiple assignments
Running checkpatch on ks7010_sdio.c shows two locations where
multiple assignment statements are used.
This patch modifies the assignments into single assignments.
Signed-off-by: Nick Rosbrook
---
drivers/staging/ks7010/ks7010_sdio.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/ks7010/ks7010_sdio.c
b/drivers/staging/ks7010/ks7010_sdio.c
index b02980d..6771cd8 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -659,10 +659,12 @@ static void ks_sdio_interrupt(struct sdio_func *func)
static int trx_device_init(struct ks_wlan_private *priv)
{
/* initialize values (tx) */
- priv->tx_dev.qtail = priv->tx_dev.qhead = 0;
+ priv->tx_dev.qhead = 0;
+ priv->tx_dev.qtail = 0;
/* initialize values (rx) */
- priv->rx_dev.qtail = priv->rx_dev.qhead = 0;
+ priv->rx_dev.qhead = 0;
+ priv->rx_dev.qtail = 0;
/* initialize spinLock (tx,rx) */
spin_lock_init(&priv->tx_dev.tx_dev_lock);
--
2.7.4
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: vt6655: card.c: Fixing multiple assignments
Running checkpath on card.c shows two locations where multiple assignments are used. This patch modifies the assignments into single assignments. Signed-off-by: Nick Rosbrook --- drivers/staging/vt6655/card.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/staging/vt6655/card.c b/drivers/staging/vt6655/card.c index 928c336..e0c9281 100644 --- a/drivers/staging/vt6655/card.c +++ b/drivers/staging/vt6655/card.c @@ -524,8 +524,11 @@ CARDvSafeResetTx( struct vnt_tx_desc *pCurrTD; /* initialize TD index */ - priv->apTailTD[0] = priv->apCurrTD[0] = &(priv->apTD0Rings[0]); - priv->apTailTD[1] = priv->apCurrTD[1] = &(priv->apTD1Rings[0]); + priv->apTailTD[0] = &(priv->apTD0Rings[0]); + priv->apCurrTD[0] = &(priv->apTD0Rings[0]); + + priv->apTailTD[1] = &(priv->apTD1Rings[0]); + priv->apCurrTD[1] = &(priv->apTD1Rings[0]); for (uu = 0; uu < TYPE_MAXTD; uu++) priv->iTDUsed[uu] = 0; -- 2.7.4 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] drivers: staging: android: Fix style/formatting issues
Fix two issues reported by checkpatch. The first removes an extra
blank line, the second changes an argument to kmalloc(). Original
checkpatch output is below:
ion/ion_cma_heap.c:34: CHECK: Please don't use multiple blank lines
ion/ion_cma_heap.c:49: CHECK: Prefer kmalloc(sizeof(*table)...) over
kmalloc(sizeof(struct sg_table)...)
total: 0 errors, 0 warnings, 2 checks, 128 lines checked
Signed-off-by: Nick Fox
---
drivers/staging/android/ion/ion_cma_heap.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index a0949bc0dcf4..bb2c1449cbcf 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -31,7 +31,6 @@ struct ion_cma_heap {
#define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap)
-
/* ION CMA heap operations functions */
static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
unsigned long len,
@@ -46,7 +45,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct
ion_buffer *buffer,
if (!pages)
return -ENOMEM;
- table = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
+ table = kmalloc(sizeof(*table), GFP_KERNEL);
if (!table)
goto err;
--
2.13.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 1/2] staging: android: ion: Remove extra line (checkpatch)
Remove extra blank line (reported by checkpatch.pl)
Signed-off-by: Nick Fox
---
drivers/staging/android/ion/ion_cma_heap.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/android/ion/ion_cma_heap.c
b/drivers/staging/android/ion/ion_cma_heap.c
index a0949bc0dcf4..b06f4cee9c7c 100644
--- a/drivers/staging/android/ion/ion_cma_heap.c
+++ b/drivers/staging/android/ion/ion_cma_heap.c
@@ -31,7 +31,6 @@ struct ion_cma_heap {
#define to_cma_heap(x) container_of(x, struct ion_cma_heap, heap)
-
/* ION CMA heap operations functions */
static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer,
unsigned long len,
--
2.13.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/2] staging: android: ion: Change argument to kmalloc (checkpatch)
Change argument to kmalloc() to fix style issue, reported by checkpatch Signed-off-by: Nick Fox --- drivers/staging/android/ion/ion_cma_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/android/ion/ion_cma_heap.c b/drivers/staging/android/ion/ion_cma_heap.c index b06f4cee9c7c..bb2c1449cbcf 100644 --- a/drivers/staging/android/ion/ion_cma_heap.c +++ b/drivers/staging/android/ion/ion_cma_heap.c @@ -45,7 +45,7 @@ static int ion_cma_allocate(struct ion_heap *heap, struct ion_buffer *buffer, if (!pages) return -ENOMEM; - table = kmalloc(sizeof(struct sg_table), GFP_KERNEL); + table = kmalloc(sizeof(*table), GFP_KERNEL); if (!table) goto err; -- 2.13.5 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: gdm724x: Rename variable for consistency
Rename dftEpsId variable to dft_eps_ID to be consistent with other
variables in the source file.
Signed-off-by: Nick Fox
---
drivers/staging/gdm724x/hci_packet.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/gdm724x/hci_packet.h
b/drivers/staging/gdm724x/hci_packet.h
index 22ce8b9477b6..d9e41e457f83 100644
--- a/drivers/staging/gdm724x/hci_packet.h
+++ b/drivers/staging/gdm724x/hci_packet.h
@@ -50,7 +50,7 @@ struct tlv {
struct sdu_header {
__dev16 cmd_evt;
__dev16 len;
- __dev32 dftEpsId;
+ __dev32 dft_eps_ID;
__dev32 bearer_ID;
__dev32 nic_type;
} __packed;
@@ -76,7 +76,7 @@ struct hci_pdn_table_ind {
__dev16 cmd_evt;
__dev16 len;
u8 activate;
- __dev32 dft_eps_id;
+ __dev32 dft_eps_ID;
__dev32 nic_type;
u8 pdn_type;
u8 ipv4_addr[4];
--
2.13.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging : gdm724x: Rename variable for consistency
v2: Undo the renaming of the dft_eps_id variable in hci_pdn_table_ind
to resolve a compiler error.
Signed-off-by: Nick Fox
---
drivers/staging/gdm724x/hci_packet.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/gdm724x/hci_packet.h
b/drivers/staging/gdm724x/hci_packet.h
index 22ce8b9477b6..f8837c7103ac 100644
--- a/drivers/staging/gdm724x/hci_packet.h
+++ b/drivers/staging/gdm724x/hci_packet.h
@@ -50,7 +50,7 @@ struct tlv {
struct sdu_header {
__dev16 cmd_evt;
__dev16 len;
- __dev32 dftEpsId;
+ __dev32 dft_eps_ID;
__dev32 bearer_ID;
__dev32 nic_type;
} __packed;
--
2.13.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: gdm724x: Rename variable for consistency
jonathan...@gctsemi.com, linux-ker...@vger.kernel.org Bcc: Subject: Re: [PATCH v2] staging : gdm724x: Rename variable for consistency Reply-To: In-Reply-To: <20170831162747.ga31...@kroah.com> Sorry for the confusion. This patch is a revision to an earlier patch I submitted that did not compile. It should be part of this thread, but in case its not, reference is here: https://lkml.org/lkml/2017/8/24/885. I thought I had compiled it successfully but I discovered this was due to an issue with my build environment. Just for future reference, was I doing something wrong in this situation by adding a v2 in the changelog area? Various beginner guides I've been reading suggested I do this when a patch requires revisions. Thanks, Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
PATCH[vme/bridges/vme_ca91cx42.c:1382: Bad if test Bug Fix]
Hey Fellow Developers,
This is my first patch so if there are any errors please reply as i will
fix them. Below is the patch.
-- drivers/vme/bridges/vme_ca91cx42.h.orig 2014-06-11 22:50:29.339671939
-0400
+++ drivers/vme/bridges/vme_ca91cx42.h 2014-06-11 23:15:36.027685173 -0400
@@ -526,7 +526,7 @@ static const int CA91CX42_LINT_LM[] = {
#define CA91CX42_VSI_CTL_SUPER_SUPR (1<<21)
#define CA91CX42_VSI_CTL_VAS_M (7<<16)
-#define CA91CX42_VSI_CTL_VAS_A16 0
+#define CA91CX42_VSI_CTL_VAS_A16 (3<<16)
#define CA91CX42_VSI_CTL_VAS_A24 (1<<16)
#define CA91CX42_VSI_CTL_VAS_A32 (1<<17)
#define CA91CX42_VSI_CTL_VAS_USER1 (3<<17)
@@ -549,7 +549,7 @@ static const int CA91CX42_LINT_LM[] = {
#define CA91CX42_LM_CTL_SUPR (1<<21)
#define CA91CX42_LM_CTL_NPRIV (1<<20)
#define CA91CX42_LM_CTL_AS_M (5<<16)
-#define CA91CX42_LM_CTL_AS_A16 0
+#define CA91CX42_LM_CTL_AS_A16 (3<<16)
#define CA91CX42_LM_CTL_AS_A24 (1<<16)
#define CA91CX42_LM_CTL_AS_A32 (1<<17)
Signed-off-by: Nicholas Krause
Nick
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Checks for Null return of skb_alloc in function fw_download_code
So sorry Greg,
I pulled the git in this morning for kernel-3.16-r2 ,I will resend the
patch from the clean tree.
I will also fix the tabs.'
Sorry :( Nick
On Wed, Jun 18, 2014 at 2:14 PM, Greg KH wrote:
> On Wed, Jun 18, 2014 at 01:53:00PM -0400, Nicholas Krause wrote:
>> Signed-off-by: Nicholas Krause
>> ---
>> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 5 -
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> index 11e915e..fde17ff 100644
>> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> @@ -62,12 +62,15 @@ static bool fw_download_code(struct net_device *dev, u8
>> *code_virtual_address,
>>
>> skb = dev_alloc_skb(frag_length + 4);
>> + if (!skb) {
>> + rt_status = false;
>> + return rt_status;
>> +
>> + }
>
> Why 2 tabs for indentation? Does that look correct?
>
>> memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
>> tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
>> tcb_desc->queue_index = TXCMD_QUEUE;
>> tcb_desc->bCmdOrInit = DESC_PACKET_TYPE_INIT;
>> tcb_desc->bLastIniPkt = bLastIniPkt;
>> - }
>>
>> seg_ptr = skb->data;
>> for (i = 0; i < frag_length; i += 4) {
>
> Also, this patch still fails to apply, what tree did you make it
> against? A "clean" 3.16-rc1 tree, or your own tree with other changes
> in it?
>
> greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192e: check return value of dev_skb_alloc
Hey guys, Sorry about no reply, my power supply died last night, have to get a new one. I will be RMAing the old old in order to get a packup in case this PSU dies. Cheers Nick On Thu, Jun 19, 2014 at 1:29 AM, Dan Carpenter wrote: > On Wed, Jun 18, 2014 at 10:24:41PM -0400, Nicholas Krause wrote: >> Checks if dev_skb_alloc returns Null in function, fw_download_code. >> If the return value of dev_skb_alloc is NULL return false and exit >> this function. >> Signed-off-by: Nicholas Krause >> --- >> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c >> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c >> index 1a95d1f..38ae236 100644 >> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c >> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c >> @@ -62,6 +62,8 @@ static bool fw_download_code(struct net_device *dev, u8 >> *code_virtual_address, >> >> skb = dev_alloc_skb(frag_length + 4); >> memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev)); >> + if (!skb) >> + return !rt_status; > > What's this !rt_status? Just return false. > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192 Check for Null return from dev_skb_alloc
Hey Guys,
So Sorry about wasting your time with bad patchs.
I feel bad about that. I hope this patch is O.K.
now. :(
Nicholas Krause
On Thu, Jun 19, 2014 at 4:18 PM, Nicholas Krause wrote:
> Checks for Null return from dev_skb_alloc if it returns Null,
> fw_download returns false. Otherwise it returns true.Also
> removed rt_status due to returning true of false directly.
>
> Signed-off-by: Nicholas Krause
> ---
> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> index 1a95d1f..affa89a 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> @@ -36,7 +36,6 @@ static bool fw_download_code(struct net_device *dev, u8
> *code_virtual_address,
> u32 buffer_len)
> {
> struct r8192_priv *priv = rtllib_priv(dev);
> - boolrt_status = true;
> u16 frag_threshold;
> u16 frag_length, frag_offset = 0;
> int i;
> @@ -59,7 +58,9 @@ static bool fw_download_code(struct net_device *dev, u8
> *code_virtual_address,
> bLastIniPkt = 1;
>
> }
> -
> + skb = dev_alloc_skb(frag_length + 4);
> + if (!skb)
> + return false;
> skb = dev_alloc_skb(frag_length + 4);
> memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
> tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
> @@ -99,7 +100,7 @@ static bool fw_download_code(struct net_device *dev, u8
> *code_virtual_address,
>
> write_nic_byte(dev, TPPoll, TPPoll_CQ);
>
> - return rt_status;
> + return false;
> }
>
> static bool CPUcheck_maincodeok_turnonCPU(struct net_device *dev)
> --
> 1.9.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192 Check for Null return from dev_skb_alloc
Thanks Joe,
I need to read my patches more carefully.
Nick :)
On Thu, Jun 19, 2014 at 4:53 PM, Joe Perches wrote:
> On Thu, 2014-06-19 at 16:20 -0400, Nick Krause wrote:
>> Hey Guys,
>> So Sorry about wasting your time with bad patchs.
>> I feel bad about that. I hope this patch is O.K.
>> now. :(
>
> Oh, one other thing...
>
>> > diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> > b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> []
>> > @@ -36,7 +36,6 @@ static bool fw_download_code(struct net_device *dev, u8
>> > *code_virtual_address,
>> > u32 buffer_len)
>> > {
>> > struct r8192_priv *priv = rtllib_priv(dev);
>> > - boolrt_status = true;
>> > u16 frag_threshold;
>> > u16 frag_length, frag_offset = 0;
>> > int i;
> []
>> > @@ -99,7 +100,7 @@ static bool fw_download_code(struct net_device *dev, u8
>> > *code_virtual_address,
>> >
>> > write_nic_byte(dev, TPPoll, TPPoll_CQ);
>> >
>> > - return rt_status;
>> > + return false;
>
> This should be:
>
> return true;
>
>
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192 Check for Null return from dev_skb_alloc
If anyone able to check this patch it would be great.
Thanks Nick
On Thu, Jun 19, 2014 at 5:18 PM, Nicholas Krause wrote:
> Checks for Null return from dev_skb_alloc if it returns Null,
> fw_download returns false. Otherwise it returns true.Also
> removed rt_status due to returning true of false directly.
>
> Signed-off-by: Nicholas Krause
> ---
> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> index 1a95d1f..affa89a 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> @@ -36,7 +36,6 @@ static bool fw_download_code(struct net_device *dev, u8
> *code_virtual_address,
> u32 buffer_len)
> {
> struct r8192_priv *priv = rtllib_priv(dev);
> - boolrt_status = true;
> u16 frag_threshold;
> u16 frag_length, frag_offset = 0;
> int i;
> @@ -59,7 +58,9 @@ static bool fw_download_code(struct net_device *dev, u8
> *code_virtual_address,
> bLastIniPkt = 1;
>
> }
>skb = dev_alloc_skb(frag_length + 4);
> + if (!skb)
> + return false;
> memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
> tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
> @@ -99,7 +100,7 @@ static bool fw_download_code(struct net_device *dev, u8
> *code_virtual_address,
>
> write_nic_byte(dev, TPPoll, TPPoll_CQ);
>
> - return rt_status;
> + return true;
> }
>
> static bool CPUcheck_maincodeok_turnonCPU(struct net_device *dev)
> --
> 1.9.1
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192 Check for Null return from dev_skb_alloc
Our you Guys just removing me from your emails I have checked this patch
and it seems right. If you don't want to accept it , the less you can
do is give me a reply.
Nick
On Thu, Jun 19, 2014 at 10:12 PM, Nick Krause wrote:
> If anyone able to check this patch it would be great.
> Thanks Nick
>
> On Thu, Jun 19, 2014 at 5:18 PM, Nicholas Krause wrote:
>> Checks for Null return from dev_skb_alloc if it returns Null,
>> fw_download returns false. Otherwise it returns true.Also
>> removed rt_status due to returning true of false directly.
>>
>> Signed-off-by: Nicholas Krause
>> ---
>> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 7 ---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> index 1a95d1f..affa89a 100644
>> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>> @@ -36,7 +36,6 @@ static bool fw_download_code(struct net_device *dev, u8
>> *code_virtual_address,
>> u32 buffer_len)
>> {
>> struct r8192_priv *priv = rtllib_priv(dev);
>> - boolrt_status = true;
>> u16 frag_threshold;
>> u16 frag_length, frag_offset = 0;
>> int i;
>> @@ -59,7 +58,9 @@ static bool fw_download_code(struct net_device *dev, u8
>> *code_virtual_address,
>> bLastIniPkt = 1;
>>
>> }
>>skb = dev_alloc_skb(frag_length + 4);
>> + if (!skb)
>> + return false;
>> memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
>> tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
>> @@ -99,7 +100,7 @@ static bool fw_download_code(struct net_device *dev, u8
>> *code_virtual_address,
>>
>> write_nic_byte(dev, TPPoll, TPPoll_CQ);
>>
>> - return rt_status;
>> + return true;
>> }
>>
>> static bool CPUcheck_maincodeok_turnonCPU(struct net_device *dev)
>> --
>> 1.9.1
>>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8192e: check return value of dev_skb_alloc
This patch is old I thought I fixed it and resent. Cheers Nick On Thu, Jun 19, 2014 at 5:25 PM, Joe Perches wrote: > On Thu, 2014-06-19 at 15:29 -0400, Nicholas Krause wrote: > >> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c >> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c > [] >> @@ -59,7 +58,8 @@ static bool fw_download_code(struct net_device *dev, u8 >> *code_virtual_address, >> bLastIniPkt = 1; >> >> } >> - >> + if (!skb) >> + return false; >> skb = dev_alloc_skb(frag_length + 4); > > Nick. > > You may have compiled this, but it's obvious you > haven't tested it. > > Please stop sending patches until you can verify what > you're doing with the code. > > Please train a few more of your neurons for coding. > > It takes some time and some practice but please don't > practice on linux-kernel. > > Use google to find some other suitable forums like > http://cboard.cprogramming.com/c-programming/ > > Back to your patch: > > if (!skb) > return false; > > must go _after_ > > skb = dev_alloc_skb(frag_length + 4); > > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:rtl8821ae: rewrite legacy wifi check in halbcoutsrc
Thanks for the feedback I will resend the patch fixed. Otherwise please use Larry's idea. Cheers Nick On Fri, Jun 20, 2014 at 4:08 PM, Joe Perches wrote: > On Fri, 2014-06-20 at 22:59 +0300, Dan Carpenter wrote: >> On Fri, Jun 20, 2014 at 12:56:50PM -0400, Nicholas Krause wrote: >> > Rewrites the wireless check for legacy checking in function >> > halbtc_legacy to check for both Mode A and B. >> >> You're just guessing that A and B were intended but it could have been >> something B and G... >> >> Don't do this. Just leave the static checker warning there so someone >> can fix it properly instead of introducing a second new bug and hiding >> the warning so it's impossible to find. >> > > It's most likely G anyway: > > drivers/staging/rtl8192ee/btcoexist/halbtcoutsrc.c: if ((mac->mode == > WIRELESS_MODE_B) || (mac->mode == WIRELESS_MODE_G)) > drivers/staging/rtl8821ae/btcoexist/halbtcoutsrc.c: if ((mac->mode == > WIRELESS_MODE_B) || (mac->mode == WIRELESS_MODE_B)) > > Larry probably has a better idea. > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging:rtl8821ae: rewrite legacy wifi check in halbcoutsrc
Is this patch being merged or is this not an issue. I am confused did I make a mistake in my patch or is there being a different patch being merged. Thank Nick On Fri, Jun 20, 2014 at 10:34 PM, Joe Perches wrote: > On Fri, 2014-06-20 at 22:26 -0400, Nick Krause wrote: >> Thanks for the feedback I will resend the patch fixed. > > Please do not. > >> Otherwise please use Larry's idea. > > It's not Larry's idea. Larry is the primary > contributor for Realtek drivers in staging. > > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Fix a Fix me in tranzport.c
On Sat, Jul 19, 2014 at 5:14 AM, Dan Carpenter wrote: > On Sat, Jul 19, 2014 at 12:09:53PM +0300, Dan Carpenter wrote: >> On Sat, Jul 19, 2014 at 01:26:52AM -0400, Nicholas Krause wrote: >> > This patch add a atomic lock in usb_tranzport_usb for preventing >> > a tiny race in this function. >> > >> > Signed-off-by: Nicholas Krause >> >> Is this a guess work patch or have you tested it? > > Nevermind. This patch doesn't even compile. > > regards, > dan carpenter > I am new so how do I compile test a directory if you can test me perhaps I can test my changes better. Cheers Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
staging: Unwritten function for ion_carveout_heap.c
Hey Greg and others. Sorry for another email but it seems the function, ion_carveout_heap_unmap_dma is just returning and not doing anything useful. Furthermore I am new so I don't known how to write this function but this may be causing some rather serious bugs as if the dma heap is not unmaped and we call this function a lot this will make the kernel not able to handle dma requests for this driver and other drivers that need this and in turn lead to a oops or even a kernel panic due to leaked dma allocated memory. I would recommend writing this function or helping me write it in other to avoid some rather serious bugs without a proper dma unmapping function for this driver :). Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: staging: Unwritten function for ion_carveout_heap.c
On Wed, Jul 23, 2014 at 4:10 PM, Colin Cross wrote: > On Wed, Jul 23, 2014 at 1:04 PM, Nick Krause wrote: >> Hey Greg and others. >> Sorry for another email but it seems the function, >> ion_carveout_heap_unmap_dma is >> just returning and not doing anything useful. Furthermore I am new so >> I don't known how >> to write this function but this may be causing some rather serious >> bugs as if the dma heap >> is not unmaped and we call this function a lot this will make the >> kernel not able to handle dma requests >> for this driver and other drivers that need this and in turn lead to >> a oops or even a kernel panic due to leaked >> dma allocated memory. I would recommend writing this function or >> helping me write it in >> other to avoid some rather serious bugs without a proper dma unmapping >> function for this driver :). >> Nick > > Look at ion_carveout_heap_map_dma - it doesn't do anything, it just > returns a pre-existing virtual address. That means there is nothing > to do in unmap. > > map_dma is actually a bit of a misnomer here, as the actual mapping is > done in ion_map_dma_buf. All ion_carveout_heap_map_dma does is return > the sg table for ion_map_dma_buf to pass to dma_map_sg. Very well then I guess this is closed then. Cheers Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 12:15 PM, Måns Rullgård wrote: > Steven Rostedt writes: > >> On Thu, 24 Jul 2014 10:47:25 -0400 >> Steven Rostedt wrote: >> >>> The three parameters are the number of elements, the size of each individual >>> element, and then finally the flags used on how to allocate that memory. >>> I have to say, you did get the flags part correct. >>> >>> Now lets look at what you did. For the size you had: >> >> That should have read "For the count you had:" >> >> Oh well, you get my point anyway. > > I have some doubts about the last bit. > > -- > Måns Rullgård > m...@mansr.com I am have this discussion with other kernel developers and just because I send out one patch as a newbie like this doesn't mean I don't known C. Cheers Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 12:34 PM, Måns Rullgård wrote: > Nick Krause writes: > >> On Thu, Jul 24, 2014 at 12:15 PM, Måns Rullgård wrote: >>> Steven Rostedt writes: >>> >>>> On Thu, 24 Jul 2014 10:47:25 -0400 >>>> Steven Rostedt wrote: >>>> >>>>> The three parameters are the number of elements, the size of each >>>>> individual >>>>> element, and then finally the flags used on how to allocate that memory. >>>>> I have to say, you did get the flags part correct. >>>>> >>>>> Now lets look at what you did. For the size you had: >>>> >>>> That should have read "For the count you had:" >>>> >>>> Oh well, you get my point anyway. >>> >>> I have some doubts about the last bit. >> >> I am have this discussion with other kernel developers and just >> because I send out one patch as a newbie like this doesn't mean I >> don't known C. > > Now I no longer have doubts; I know for certain that the point didn't > get across. > > -- > Måns Rullgård > m...@mansr.com I get your point try other things but I talked to other kernel developers and most of them seemed to get this feedback to when they started. Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 12:47 PM, Måns Rullgård wrote: > Nick Krause writes: > >> On Thu, Jul 24, 2014 at 12:34 PM, Måns Rullgård wrote: >>> Nick Krause writes: >>> >>>> On Thu, Jul 24, 2014 at 12:15 PM, Måns Rullgård wrote: >>>>> Steven Rostedt writes: >>>>> >>>>>> On Thu, 24 Jul 2014 10:47:25 -0400 >>>>>> Steven Rostedt wrote: >>>>>> >>>>>>> The three parameters are the number of elements, the size of each >>>>>>> individual >>>>>>> element, and then finally the flags used on how to allocate that memory. >>>>>>> I have to say, you did get the flags part correct. >>>>>>> >>>>>>> Now lets look at what you did. For the size you had: >>>>>> >>>>>> That should have read "For the count you had:" >>>>>> >>>>>> Oh well, you get my point anyway. >>>>> >>>>> I have some doubts about the last bit. >>>> >>>> I am have this discussion with other kernel developers and just >>>> because I send out one patch as a newbie like this doesn't mean I >>>> don't known C. >>> >>> Now I no longer have doubts; I know for certain that the point didn't >>> get across. >> >> I get your point try other things but I talked to other kernel >> developers and most of them seemed to get this feedback to when they >> started. > > Most kernel devs most certainly did NOT get started by spamming lkml > with unnecessary and incorrect patches despite being repeatedly told to > go away and stop wasting everybody's time. > > -- > Måns Rullgård > m...@mansr.com FIne , I give up. You want me to leave this then I wiil. Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 1:48 PM, Steven Rostedt wrote: > On Thu, 24 Jul 2014 10:30:51 -0700 > Harvey Harrison wrote: > >> On Thu, Jul 24, 2014 at 10:18 AM, Steven Rostedt wrote: >> > On Thu, 24 Jul 2014 12:50:31 -0400 >> > Nick Krause wrote: >> >> > >> >> I am have this discussion with other kernel developers and just >> >> because I send out one patch as a newbie like this doesn't mean I >> >> don't known C. >> > >> > It's not just one patch, and I didn't say you don't know C. I said you >> > don't understand C enough for kernel development. >> >> And more importantly, stop guessing things are OK and selfishly asking >> others to check >> your work for you. >> >> If you have not at least _built_ the kernel with your change, and not >> _run_ it, and not made >> sure that the changed code is being _run_...you are wasting other people's >> time. >> > > Nick, > > Just to bring Harvey's email here in perspective. I mentioned in my > previous email that most of us are overworked. We don't have time to > teach you if your patches are correct or not. By saying, "I'll just > write the code and you tell me if it's correct, and test it for me" is > like telling someone "Here's a lawn mower, now go mow my lawn". > > We like it if people put effort into their work and are confident > themselves that they have the right code or not. There's enough > reviewing of patches to do from people that put that effort in, that > there's no time to review patches from people that don't know if their > work or not. > > Cheers, > > -- Steve Steve, I have programming a lot in other areas just not the kernel. You are right through I need to test my code better through. Cheers Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 2:49 PM, Richard Weinberger wrote: > On Thu, Jul 24, 2014 at 8:41 PM, Nick Krause wrote: >> Steve, >> I have programming a lot in other areas just not the kernel. >> You are right through I need to test my code better through. > > Nick, > > let's make a deal. > Take the challenge at http://eudyptula-challenge.org. After you've solved all > tasks we'll accept patches from you. > > -- > Thanks, > //richard That's fine, So Sorry :(. Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 2:55 PM, Nick Krause wrote: > On Thu, Jul 24, 2014 at 2:49 PM, Richard Weinberger > wrote: >> On Thu, Jul 24, 2014 at 8:41 PM, Nick Krause wrote: >>> Steve, >>> I have programming a lot in other areas just not the kernel. >>> You are right through I need to test my code better through. >> >> Nick, >> >> let's make a deal. >> Take the challenge at http://eudyptula-challenge.org. After you've solved all >> tasks we'll accept patches from you. >> >> -- >> Thanks, >> //richard > That's fine, > So Sorry :(. > Nick I sent the challenge a email. Seems that there not replying. Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 10:15 PM, Chen Gang wrote: > > Excuse me, I did not see the patch details, but I guess it is about > whether welcome a new member to upstream kernel. For me, I have 3 ideas > about it if I am a newbie or a normal kernel developer: > > - Do I have enough basic skills for it? > > - Do I developed one or more another real world projects with C? > (they are successful projects -- at least, not failure projects) > > - Can I construct related develop environments to support what I do? > (e.g. building and testing for upstream kernel) > > - Am I familiar the basic working flow about kernel mailing list. > (e.g. format-patch, sending patch, email client configuration...). > > - Do I have a correct attitude on it? > > - Am I careful enough? > (e.g. if find some details may doubt, try to check and clear them) > (it is always neccessary for all patches) > > - Am I try my best for it? > (e.g. when finish coding, try what I can do: build, run and test). > (it is neccessary for most cases) > > - Do I have negative effect with others? > (e.g. discourage the newbies, or send spam to other members ...). > (need always try to avoid) > > - Do I really love programming, also love open source kernel? > > - Do I love it, or I have to do it for another reason? > (I guess, most of gmail members in open source, love programming). > > - Do I still love programming if I am discouraged by any members? > > - Do I still love open source kernel if discouraged by any members? > > I have most of these skills other then developing real projects in C with other developers as other then the kernel I don't have must interest in C outside of embedded programming. Cheers Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 10:47 PM, Chen Gang wrote: > > > On 07/25/2014 10:20 AM, Nick Krause wrote: >> On Thu, Jul 24, 2014 at 10:15 PM, Chen Gang wrote: >>> >>> Excuse me, I did not see the patch details, but I guess it is about >>> whether welcome a new member to upstream kernel. For me, I have 3 ideas >>> about it if I am a newbie or a normal kernel developer: >>> >>> - Do I have enough basic skills for it? >>> >>> - Do I developed one or more another real world projects with C? >>> (they are successful projects -- at least, not failure projects) >>> >>> - Can I construct related develop environments to support what I do? >>> (e.g. building and testing for upstream kernel) >>> >>> - Am I familiar the basic working flow about kernel mailing list. >>> (e.g. format-patch, sending patch, email client configuration...). >>> >>> - Do I have a correct attitude on it? >>> >>> - Am I careful enough? >>> (e.g. if find some details may doubt, try to check and clear them) >>> (it is always neccessary for all patches) >>> >>> - Am I try my best for it? >>> (e.g. when finish coding, try what I can do: build, run and test). >>> (it is neccessary for most cases) >>> >>> - Do I have negative effect with others? >>> (e.g. discourage the newbies, or send spam to other members ...). >>> (need always try to avoid) >>> >>> - Do I really love programming, also love open source kernel? >>> >>> - Do I love it, or I have to do it for another reason? >>> (I guess, most of gmail members in open source, love programming). >>> >>> - Do I still love programming if I am discouraged by any members? >>> >>> - Do I still love open source kernel if discouraged by any members? >>> >>> >> I have most of these skills other then developing real projects in C with >> other >> developers as other then the kernel I don't have must interest in C outside >> of embedded programming. > > OK, thanks. If what you said is true, for me, if you will, please still > continue for open source kernel, but really need be more careful about > the patch you made before send to open mailing list. > > > Thanks. > -- > Chen Gang > > Open, share, and attitude like air, water, and life which God blessed Chen , Thanks for helping me out here :). The true through is I don't known how to build test correctly as a newb and thought it was better just to avoid it(stupid me). I would like to known how to do this and if there is any way I can help you guys out , please let me known :). I don't want to be avoided I am honestly here to help make your life easier. NIck ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 11:09 PM, Chen Gang wrote: > > > On 07/25/2014 10:53 AM, Nick Krause wrote: >> On Thu, Jul 24, 2014 at 10:47 PM, Chen Gang wrote: >>> >>> >>> On 07/25/2014 10:20 AM, Nick Krause wrote: >>>> On Thu, Jul 24, 2014 at 10:15 PM, Chen Gang >>>> wrote: >>>>> >>>>> Excuse me, I did not see the patch details, but I guess it is about >>>>> whether welcome a new member to upstream kernel. For me, I have 3 ideas >>>>> about it if I am a newbie or a normal kernel developer: >>>>> >>>>> - Do I have enough basic skills for it? >>>>> >>>>> - Do I developed one or more another real world projects with C? >>>>> (they are successful projects -- at least, not failure projects) >>>>> >>>>> - Can I construct related develop environments to support what I do? >>>>> (e.g. building and testing for upstream kernel) >>>>> >>>>> - Am I familiar the basic working flow about kernel mailing list. >>>>> (e.g. format-patch, sending patch, email client configuration...). >>>>> >>>>> - Do I have a correct attitude on it? >>>>> >>>>> - Am I careful enough? >>>>> (e.g. if find some details may doubt, try to check and clear them) >>>>> (it is always neccessary for all patches) >>>>> >>>>> - Am I try my best for it? >>>>> (e.g. when finish coding, try what I can do: build, run and test). >>>>> (it is neccessary for most cases) >>>>> >>>>> - Do I have negative effect with others? >>>>> (e.g. discourage the newbies, or send spam to other members ...). >>>>> (need always try to avoid) >>>>> >>>>> - Do I really love programming, also love open source kernel? >>>>> >>>>> - Do I love it, or I have to do it for another reason? >>>>> (I guess, most of gmail members in open source, love programming). >>>>> >>>>> - Do I still love programming if I am discouraged by any members? >>>>> >>>>> - Do I still love open source kernel if discouraged by any members? >>>>> >>>>> >>>> I have most of these skills other then developing real projects in C with >>>> other >>>> developers as other then the kernel I don't have must interest in C outside >>>> of embedded programming. >>> >>> OK, thanks. If what you said is true, for me, if you will, please still >>> continue for open source kernel, but really need be more careful about >>> the patch you made before send to open mailing list. >>> >>> >>> Thanks. >>> -- >>> Chen Gang >>> >>> Open, share, and attitude like air, water, and life which God blessed >> >> Chen , >> Thanks for helping me out here :). The true through is I don't known how to >> build test correctly as a newb and thought it was better just to avoid >> it(stupid >> me). I would like to known how to do this and if there is any way I can help >> you >> guys out , please let me known :). I don't want to be avoided I am honestly >> here >> to help make your life easier. > > Excuse me, my English is not quite well, I do not understand what you > said, I guess your meaning is "not familiar with how to build and test > upstream kernel" If what I guess is incorrect, please let me know. > > There are some books or some related web links for building and test > kernel, they are not quite difficult to get (please google search). > > I guess, it is necessary to reference these information and construct > related develop environments to support what you want to do next. > > > By the way, excuse me, today I have to do some other things firstly, so > maybe my next email reply will be late (may tomorrow, or the day after > tomorrow). > > > Thanks. > -- > Chen Gang > > Open, share, and attitude like air, water, and life which God blessed That's what I meant. If you can sent me a link to how to do build testing that would be great :). Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 11:21 PM, Chen Gang wrote: > > > On 07/25/2014 11:13 AM, Nick Krause wrote: >> On Thu, Jul 24, 2014 at 11:09 PM, Chen Gang wrote: >>> >>> >>> On 07/25/2014 10:53 AM, Nick Krause wrote: >>>> On Thu, Jul 24, 2014 at 10:47 PM, Chen Gang >>>> wrote: >>>>> >>>>> >>>>> On 07/25/2014 10:20 AM, Nick Krause wrote: >>>>>> On Thu, Jul 24, 2014 at 10:15 PM, Chen Gang >>>>>> wrote: >>>>>>> >>>>>>> Excuse me, I did not see the patch details, but I guess it is about >>>>>>> whether welcome a new member to upstream kernel. For me, I have 3 ideas >>>>>>> about it if I am a newbie or a normal kernel developer: >>>>>>> >>>>>>> - Do I have enough basic skills for it? >>>>>>> >>>>>>> - Do I developed one or more another real world projects with C? >>>>>>> (they are successful projects -- at least, not failure projects) >>>>>>> >>>>>>> - Can I construct related develop environments to support what I do? >>>>>>> (e.g. building and testing for upstream kernel) >>>>>>> >>>>>>> - Am I familiar the basic working flow about kernel mailing list. >>>>>>> (e.g. format-patch, sending patch, email client configuration...). >>>>>>> >>>>>>> - Do I have a correct attitude on it? >>>>>>> >>>>>>> - Am I careful enough? >>>>>>> (e.g. if find some details may doubt, try to check and clear them) >>>>>>> (it is always neccessary for all patches) >>>>>>> >>>>>>> - Am I try my best for it? >>>>>>> (e.g. when finish coding, try what I can do: build, run and test). >>>>>>> (it is neccessary for most cases) >>>>>>> >>>>>>> - Do I have negative effect with others? >>>>>>> (e.g. discourage the newbies, or send spam to other members ...). >>>>>>> (need always try to avoid) >>>>>>> >>>>>>> - Do I really love programming, also love open source kernel? >>>>>>> >>>>>>> - Do I love it, or I have to do it for another reason? >>>>>>> (I guess, most of gmail members in open source, love programming). >>>>>>> >>>>>>> - Do I still love programming if I am discouraged by any members? >>>>>>> >>>>>>> - Do I still love open source kernel if discouraged by any members? >>>>>>> >>>>>>> >>>>>> I have most of these skills other then developing real projects in C >>>>>> with other >>>>>> developers as other then the kernel I don't have must interest in C >>>>>> outside >>>>>> of embedded programming. >>>>> >>>>> OK, thanks. If what you said is true, for me, if you will, please still >>>>> continue for open source kernel, but really need be more careful about >>>>> the patch you made before send to open mailing list. >>>>> >>>>> >>>>> Thanks. >>>>> -- >>>>> Chen Gang >>>>> >>>>> Open, share, and attitude like air, water, and life which God blessed >>>> >>>> Chen , >>>> Thanks for helping me out here :). The true through is I don't known how to >>>> build test correctly as a newb and thought it was better just to avoid >>>> it(stupid >>>> me). I would like to known how to do this and if there is any way I can >>>> help you >>>> guys out , please let me known :). I don't want to be avoided I am >>>> honestly here >>>> to help make your life easier. >>> >>> Excuse me, my English is not quite well, I do not understand what you >>> said, I guess your meaning is "not familiar with how to build and test >>> upstream kernel" If what I guess is incorrect, please let me know. >>> >>> There are some books or some related web links for building and test >>> kernel, they are not quite difficult to get (please google search). >>> >>> I guess, it is necessary to reference these information and construct >>> related develop environments to support what you want to do next. >>> >>> >>> By the way, excuse me, today I have to do some other things firstly, so >>> maybe my next email reply will be late (may tomorrow, or the day after >>> tomorrow). >>> >>> >>> Thanks. >>> -- >>> Chen Gang >>> >>> Open, share, and attitude like air, water, and life which God blessed >> >> That's what I meant. If you can sent me a link to how to do build testing >> that would be great :). > > Please read README at the root directory of kernel source, also > "./Documents" contents many valuable information. And try google search > for additional information. > > Try to accomplish them by yourself, that will let yourself improved with > efficiency. > > Thanks. > -- > Chen Gang > > Open, share, and attitude like air, water, and life which God blessed That seems to be the issue with my programming here. Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Change kzalloc to kcalloc
On Thu, Jul 24, 2014 at 11:34 PM, Chen Gang wrote: > > > On 07/25/2014 11:30 AM, Nick Krause wrote: >> On Thu, Jul 24, 2014 at 11:21 PM, Chen Gang wrote: >>> >>> >>> On 07/25/2014 11:13 AM, Nick Krause wrote: >>>> On Thu, Jul 24, 2014 at 11:09 PM, Chen Gang >>>> wrote: >>>>> >>>>> >>>>> On 07/25/2014 10:53 AM, Nick Krause wrote: >>>>>> On Thu, Jul 24, 2014 at 10:47 PM, Chen Gang >>>>>> wrote: >>>>>>> >>>>>>> >>>>>>> On 07/25/2014 10:20 AM, Nick Krause wrote: >>>>>>>> On Thu, Jul 24, 2014 at 10:15 PM, Chen Gang >>>>>>>> wrote: >>>>>>>>> >>>>>>>>> Excuse me, I did not see the patch details, but I guess it is about >>>>>>>>> whether welcome a new member to upstream kernel. For me, I have 3 >>>>>>>>> ideas >>>>>>>>> about it if I am a newbie or a normal kernel developer: >>>>>>>>> >>>>>>>>> - Do I have enough basic skills for it? >>>>>>>>> >>>>>>>>> - Do I developed one or more another real world projects with C? >>>>>>>>> (they are successful projects -- at least, not failure projects) >>>>>>>>> >>>>>>>>> - Can I construct related develop environments to support what I >>>>>>>>> do? >>>>>>>>> (e.g. building and testing for upstream kernel) >>>>>>>>> >>>>>>>>> - Am I familiar the basic working flow about kernel mailing list. >>>>>>>>> (e.g. format-patch, sending patch, email client >>>>>>>>> configuration...). >>>>>>>>> >>>>>>>>> - Do I have a correct attitude on it? >>>>>>>>> >>>>>>>>> - Am I careful enough? >>>>>>>>> (e.g. if find some details may doubt, try to check and clear >>>>>>>>> them) >>>>>>>>> (it is always neccessary for all patches) >>>>>>>>> >>>>>>>>> - Am I try my best for it? >>>>>>>>> (e.g. when finish coding, try what I can do: build, run and >>>>>>>>> test). >>>>>>>>> (it is neccessary for most cases) >>>>>>>>> >>>>>>>>> - Do I have negative effect with others? >>>>>>>>> (e.g. discourage the newbies, or send spam to other members >>>>>>>>> ...). >>>>>>>>> (need always try to avoid) >>>>>>>>> >>>>>>>>> - Do I really love programming, also love open source kernel? >>>>>>>>> >>>>>>>>> - Do I love it, or I have to do it for another reason? >>>>>>>>> (I guess, most of gmail members in open source, love >>>>>>>>> programming). >>>>>>>>> >>>>>>>>> - Do I still love programming if I am discouraged by any members? >>>>>>>>> >>>>>>>>> - Do I still love open source kernel if discouraged by any >>>>>>>>> members? >>>>>>>>> >>>>>>>>> >>>>>>>> I have most of these skills other then developing real projects in C >>>>>>>> with other >>>>>>>> developers as other then the kernel I don't have must interest in C >>>>>>>> outside >>>>>>>> of embedded programming. >>>>>>> >>>>>>> OK, thanks. If what you said is true, for me, if you will, please still >>>>>>> continue for open source kernel, but really need be more careful about >>>>>>> the patch you made before send to open mailing list. >>>>>>> >>>>>>> >>>>>>> Thanks. >>>>>>> -- >>>>>>> Chen Gang >>>>>>> >>>>>>> Open, share, and attitude like air, water, and life
Re: [PATCH] staging: Check against NULL in fw_download_code
On Mon, Aug 11, 2014 at 2:02 PM, Nicholas Krause wrote:
> I am fixing the bug entry , https://bugzilla.kernel.org/show_bug.cgi?id=60461.
> This entry states that we are not checking the skb allocated in
> fw_download_code
> and after checking I fixed it to check for the NULL value before using the
> allocate
> skb.
>
> Signed-off-by: Nicholas Krause
> ---
> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 14 --
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> index 1a95d1f..0a4c926 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> @@ -60,13 +60,15 @@ static bool fw_download_code(struct net_device *dev, u8
> *code_virtual_address,
>
> }
>
> - skb = dev_alloc_skb(frag_length + 4);
> - memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
> - tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
> - tcb_desc->queue_index = TXCMD_QUEUE;
> - tcb_desc->bCmdOrInit = DESC_PACKET_TYPE_INIT;
> - tcb_desc->bLastIniPkt = bLastIniPkt;
>
> + skb = dev_alloc_skb(frag_length + 4);
> + if (skb) {
> + memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
> + tcb_desc = (struct cb_desc *)(skb->cb +
> MAX_DEV_ADDR_SIZE);
> + tcb_desc->queue_index = TXCMD_QUEUE;
> + tcb_desc->bCmdOrInit = DESC_PACKET_TYPE_INIT;
> + tcb_desc->bLastIniPkt = bLastIniPkt;
> + }
> seg_ptr = skb->data;
> for (i = 0; i < frag_length; i += 4) {
> *seg_ptr++ = ((i+0) < frag_length) ?
> --
> 1.9.1
>
And I did check it against Linus's tree to make sure it applies , just
to let you known.
Nick
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Check against NULL in fw_download_code
On Mon, Aug 11, 2014 at 2:07 PM, Randy Dunlap wrote:
> On 08/11/14 11:04, Nick Krause wrote:
>> On Mon, Aug 11, 2014 at 2:02 PM, Nicholas Krause wrote:
>>> I am fixing the bug entry ,
>>> https://bugzilla.kernel.org/show_bug.cgi?id=60461.
>>> This entry states that we are not checking the skb allocated in
>>> fw_download_code
>>> and after checking I fixed it to check for the NULL value before using the
>>> allocate
>>> skb.
>>>
>>> Signed-off-by: Nicholas Krause
>>> ---
>>> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 14 --
>>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>> index 1a95d1f..0a4c926 100644
>>> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>> @@ -60,13 +60,15 @@ static bool fw_download_code(struct net_device *dev, u8
>>> *code_virtual_address,
>>>
>>> }
>>>
>>> - skb = dev_alloc_skb(frag_length + 4);
>>> - memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
>>> - tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
>>> - tcb_desc->queue_index = TXCMD_QUEUE;
>>> - tcb_desc->bCmdOrInit = DESC_PACKET_TYPE_INIT;
>>> - tcb_desc->bLastIniPkt = bLastIniPkt;
>>>
>>> + skb = dev_alloc_skb(frag_length + 4);
>>> + if (skb) {
>>> + memcpy((unsigned char *)(skb->cb), &dev,
>>> sizeof(dev));
>>> + tcb_desc = (struct cb_desc *)(skb->cb +
>>> MAX_DEV_ADDR_SIZE);
>>> + tcb_desc->queue_index = TXCMD_QUEUE;
>>> + tcb_desc->bCmdOrInit = DESC_PACKET_TYPE_INIT;
>>> + tcb_desc->bLastIniPkt = bLastIniPkt;
>>> + }
>
> and what happens here (below) if skb is NULL?
>
>>> seg_ptr = skb->data;
>>> for (i = 0; i < frag_length; i += 4) {
>>> *seg_ptr++ = ((i+0) < frag_length) ?
>>> --
>>> 1.9.1
>>>
>> And I did check it against Linus's tree to make sure it applies , just
>> to let you known.
>> Nick
>
>
> --
> ~Randy
Sorry Randy.
I may be mis reading this, but are you asking me to write a different
commit message or is this patch just another bad patch in my series of
bad patches?
Nick
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Check against NULL in fw_download_code
On Mon, Aug 11, 2014 at 2:45 PM, Randy Dunlap wrote:
> On 08/11/14 11:26, Nick Krause wrote:
>> On Mon, Aug 11, 2014 at 2:07 PM, Randy Dunlap wrote:
>>> On 08/11/14 11:04, Nick Krause wrote:
>>>> On Mon, Aug 11, 2014 at 2:02 PM, Nicholas Krause
>>>> wrote:
>>>>> I am fixing the bug entry ,
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=60461.
>>>>> This entry states that we are not checking the skb allocated in
>>>>> fw_download_code
>>>>> and after checking I fixed it to check for the NULL value before using
>>>>> the allocate
>>>>> skb.
>>>>>
>>>>> Signed-off-by: Nicholas Krause
>>>>> ---
>>>>> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 14 --
>>>>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>>>> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>>>> index 1a95d1f..0a4c926 100644
>>>>> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>>>> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>>>> @@ -60,13 +60,15 @@ static bool fw_download_code(struct net_device *dev,
>>>>> u8 *code_virtual_address,
>>>>>
>>>>> }
>>>>>
>>>>> - skb = dev_alloc_skb(frag_length + 4);
>>>>> - memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
>>>>> - tcb_desc = (struct cb_desc *)(skb->cb +
>>>>> MAX_DEV_ADDR_SIZE);
>>>>> - tcb_desc->queue_index = TXCMD_QUEUE;
>>>>> - tcb_desc->bCmdOrInit = DESC_PACKET_TYPE_INIT;
>>>>> - tcb_desc->bLastIniPkt = bLastIniPkt;
>>>>>
>>>>> + skb = dev_alloc_skb(frag_length + 4);
>>>>> + if (skb) {
>>>>> + memcpy((unsigned char *)(skb->cb), &dev,
>>>>> sizeof(dev));
>>>>> + tcb_desc = (struct cb_desc *)(skb->cb +
>>>>> MAX_DEV_ADDR_SIZE);
>>>>> + tcb_desc->queue_index = TXCMD_QUEUE;
>>>>> + tcb_desc->bCmdOrInit = DESC_PACKET_TYPE_INIT;
>>>>> + tcb_desc->bLastIniPkt = bLastIniPkt;
>>>>> + }
>>>
>>> and what happens here (below) if skb is NULL?
>
> Nick,
> I'm asking if you have completely fixed the bug or only partially fixed it.
> The answer is that the patch is only a partial fix. If skb is NULL, there
> is still a problem in the statement below here. The kernel will oops on
> that reference to skb, which is NULL.
>
>>>
>>>>> seg_ptr = skb->data;
>>>>> for (i = 0; i < frag_length; i += 4) {
>>>>> *seg_ptr++ = ((i+0) < frag_length) ?
>>>>> --
>>>>> 1.9.1
>>>>>
>>>> And I did check it against Linus's tree to make sure it applies , just
>>>> to let you known.
>>>> Nick
>>>
>>>
>>> --
>>> ~Randy
>> Sorry Randy.
>> I may be mis reading this, but are you asking me to write a different
>> commit message or is this patch just another bad patch in my series of
>> bad patches?
>
>
> --
> ~Randy
Randy ,
Thanks for the catch :). Would you recommend just putting it in the if
statement I created or create a second if statement for
code readability.
Cheers Nick
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Check against NULL in fw_download_code
On Mon, Aug 11, 2014 at 3:04 PM, Randy Dunlap wrote:
> On 08/11/14 11:55, Nick Krause wrote:
>> On Mon, Aug 11, 2014 at 2:45 PM, Randy Dunlap wrote:
>>> On 08/11/14 11:26, Nick Krause wrote:
>>>> On Mon, Aug 11, 2014 at 2:07 PM, Randy Dunlap
>>>> wrote:
>>>>> On 08/11/14 11:04, Nick Krause wrote:
>>>>>> On Mon, Aug 11, 2014 at 2:02 PM, Nicholas Krause
>>>>>> wrote:
>>>>>>> I am fixing the bug entry ,
>>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=60461.
>>>>>>> This entry states that we are not checking the skb allocated in
>>>>>>> fw_download_code
>>>>>>> and after checking I fixed it to check for the NULL value before using
>>>>>>> the allocate
>>>>>>> skb.
>>>>>>>
>>>>>>> Signed-off-by: Nicholas Krause
>>>>>>> ---
>>>>>>> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 14 --
>>>>>>> 1 file changed, 8 insertions(+), 6 deletions(-)
>>>>>>>
>>>>>>> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>>>>>> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>>>>>> index 1a95d1f..0a4c926 100644
>>>>>>> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>>>>>> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
>>>>>>> @@ -60,13 +60,15 @@ static bool fw_download_code(struct net_device
>>>>>>> *dev, u8 *code_virtual_address,
>>>>>>>
>>>>>>> }
>>>>>>>
>>>>>>> - skb = dev_alloc_skb(frag_length + 4);
>>>>>>> - memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
>>>>>>> - tcb_desc = (struct cb_desc *)(skb->cb +
>>>>>>> MAX_DEV_ADDR_SIZE);
>>>>>>> - tcb_desc->queue_index = TXCMD_QUEUE;
>>>>>>> - tcb_desc->bCmdOrInit = DESC_PACKET_TYPE_INIT;
>>>>>>> - tcb_desc->bLastIniPkt = bLastIniPkt;
>>>>>>>
>>>>>>> + skb = dev_alloc_skb(frag_length + 4);
>>>>>>> + if (skb) {
>>>>>>> + memcpy((unsigned char *)(skb->cb), &dev,
>>>>>>> sizeof(dev));
>>>>>>> + tcb_desc = (struct cb_desc *)(skb->cb +
>>>>>>> MAX_DEV_ADDR_SIZE);
>>>>>>> + tcb_desc->queue_index = TXCMD_QUEUE;
>>>>>>> + tcb_desc->bCmdOrInit = DESC_PACKET_TYPE_INIT;
>>>>>>> + tcb_desc->bLastIniPkt = bLastIniPkt;
>>>>>>> + }
>>>>>
>>>>> and what happens here (below) if skb is NULL?
>>>
>>> Nick,
>>> I'm asking if you have completely fixed the bug or only partially fixed it.
>>> The answer is that the patch is only a partial fix. If skb is NULL, there
>>> is still a problem in the statement below here. The kernel will oops on
>>> that reference to skb, which is NULL.
>>>
>>>>>
>>>>>>> seg_ptr = skb->data;
>>>>>>> for (i = 0; i < frag_length; i += 4) {
>>>>>>> *seg_ptr++ = ((i+0) < frag_length) ?
>>>>>>> --
>>>>>>> 1.9.1
>>>>>>>
>>>>>> And I did check it against Linus's tree to make sure it applies , just
>>>>>> to let you known.
>>>>>> Nick
>>>>>
>>>>>
>>>>> --
>>>>> ~Randy
>>>> Sorry Randy.
>>>> I may be mis reading this, but are you asking me to write a different
>>>> commit message or is this patch just another bad patch in my series of
>>>> bad patches?
>>>
>>>
>>> --
>>> ~Randy
>>
>> Randy ,
>> Thanks for the catch :). Would you recommend just putting it in the if
>> statement I created or create a second if statement for
>> code readability.
>
> Neither one of those choices. I suggest that the code immediately check for
> skb == NULL and return 0 (or false) if it is NULL. Then the code below that
> check
> won't need to be changed at all.
>
>
>
> --
> ~Randy
Thanks Randy for your help otherwise is my patch good? Please let me
known if there our any other issues.
Regards Nick
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: Check against NULL in fw_download_code
Thanks Randy for checking this I am going to resend as a v2 patch. Regards Nick On Mon, Aug 11, 2014 at 3:40 PM, Randy Dunlap wrote: > On 08/11/14 12:18, Nick Krause wrote: >> >> Thanks Randy for your help otherwise is my patch good? Please let me >> known if there our any other issues. >> Regards Nick > > I didn't notice any other issues with it. > > -- > ~Randy ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv2] staging: Check against NULL in fw_download_code
On Mon, Aug 11, 2014 at 5:48 PM, Jerry Snitselaar wrote: > On Mon Aug 11 14, Nicholas Krause wrote: >> I am fixing the bug entry , >> https://bugzilla.kernel.org/show_bug.cgi?id=60461. >> This entry states that we are not checking the skb allocated in >> fw_download_code >> and after checking I fixed it to check for the NULL value before using the >> allocate >> skb. >> >> Signed-off-by: Nicholas Krause >> --- >> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c >> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c >> index 1a95d1f..817e50e 100644 >> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c >> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c >> @@ -61,6 +61,8 @@ static bool fw_download_code(struct net_device *dev, u8 >> *code_virtual_address, >> } >> >> skb = dev_alloc_skb(frag_length + 4); >> + if (skb == NULL) >> + return !rt_status; >> memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev)); >> tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE); >> tcb_desc->queue_index = TXCMD_QUEUE; >> -- >> 1.9.1 > > > Look at fw_download_code() in drivers/staging/rtl8192u/r819xU_firmware.c Hey Jerry, I assume you want me to return true or false directly and remove rt_status. Cheers Nick ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv3] staging: Check for Null allocated skb in fw_download_code
On Mon, Aug 11, 2014 at 7:12 PM, Nicholas Krause wrote:
> I am fixing the bug entry , https://bugzilla.kernel.org/show_bug.cgi?id=60461.
> This entry states that we are not checking the skb allocated in
> fw_download_code
> for NULL and after checking it ,I fixed it to check for the NULL value before
> returning false and exiting fw_download_code cleanly. In additon I removed the
> variable, rt_status as it's easier to read this function's return value with
> just true or false and rt status is a unneeded variable for the bool return
> of this function.
>
> Signed-off-by: Nicholas Krause
> ---
> drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> index 1a95d1f..66d83f8 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/r8192E_firmware.c
> @@ -36,7 +36,6 @@ static bool fw_download_code(struct net_device *dev, u8
> *code_virtual_address,
> u32 buffer_len)
> {
> struct r8192_priv *priv = rtllib_priv(dev);
> - boolrt_status = true;
> u16 frag_threshold;
> u16 frag_length, frag_offset = 0;
> int i;
> @@ -61,6 +60,8 @@ static bool fw_download_code(struct net_device *dev, u8
> *code_virtual_address,
> }
>
> skb = dev_alloc_skb(frag_length + 4);
> + if (skb == NULL)
> + return false;
> memcpy((unsigned char *)(skb->cb), &dev, sizeof(dev));
> tcb_desc = (struct cb_desc *)(skb->cb + MAX_DEV_ADDR_SIZE);
> tcb_desc->queue_index = TXCMD_QUEUE;
> @@ -99,7 +100,7 @@ static bool fw_download_code(struct net_device *dev, u8
> *code_virtual_address,
>
> write_nic_byte(dev, TPPoll, TPPoll_CQ);
>
> - return rt_status;
> + return true;
> }
>
> static bool CPUcheck_maincodeok_turnonCPU(struct net_device *dev)
> --
> 1.9.1
>
Sorry about the three versions, other people wanted to make the code
and my bug fix correct.
Cheers Nick
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
