[PATCH 0/2] mmc: trivial patches to fix sparse warnings

[PATCH 0/2] mmc: trivial patches to fix sparse warnings

[Venkatraman S]

Couple of patches to cleanup warnings generated by sparse tool.

Venkatraman S (2):
  mmc: queue: declare mmc_alloc_sg as static
  mmc: fix integer assignments to pointer

 drivers/mmc/card/block.c    |    4 ++--
 drivers/mmc/card/queue.c    |    2 +-
 drivers/mmc/core/core.c     |    2 +-
 drivers/mmc/core/mmc_ops.c  |    4 ++--
 drivers/mmc/core/sdio_ops.c |    2 +-
 5 files changed, 7 insertions(+), 7 deletions(-)

[PATCH 1/2] mmc: queue: declare mmc_alloc_sg as static

[Venkatraman S]

Fix the sparse warning "drivers/mmc/card/queue.c:111:20: warning:
 symbol 'mmc_alloc_sg' was not declared. Should it be static?"

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/card/queue.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/card/queue.c b/drivers/mmc/card/queue.c
index 45fb362..5196312 100644
--- a/drivers/mmc/card/queue.c
+++ b/drivers/mmc/card/queue.c
@@ -108,7 +108,7 @@ static void mmc_request(struct request_queue *q)
 		wake_up_process(mq->thread);
 }
 
-struct scatterlist *mmc_alloc_sg(int sg_len, int *err)
+static struct scatterlist *mmc_alloc_sg(int sg_len, int *err)
 {
 	struct scatterlist *sg;
 
-- 
1.7.1

[PATCH 2/2] mmc: fix integer assignments to pointer

[Venkatraman S]

Fix the sparse warning output
"warning: Using plain integer as NULL pointer"

Signed-off-by: Venkatraman S <svenkatr@ti.com>
---
 drivers/mmc/card/block.c    |    4 ++--
 drivers/mmc/core/core.c     |    2 +-
 drivers/mmc/core/mmc_ops.c  |    4 ++--
 drivers/mmc/core/sdio_ops.c |    2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 1ff5486..e702c61 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -291,7 +291,7 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
 	struct mmc_card *card;
 	struct mmc_command cmd = {0};
 	struct mmc_data data = {0};
-	struct mmc_request mrq = {0};
+	struct mmc_request mrq = {NULL};
 	struct scatterlist sg;
 	int err;
 
@@ -466,7 +466,7 @@ static u32 mmc_sd_num_wr_blocks(struct mmc_card *card)
 	u32 result;
 	__be32 *blocks;
 
-	struct mmc_request mrq = {0};
+	struct mmc_request mrq = {NULL};
 	struct mmc_command cmd = {0};
 	struct mmc_data data = {0};
 	unsigned int timeout_us;
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 89bdeae..11fcf20 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -330,7 +330,7 @@ EXPORT_SYMBOL(mmc_wait_for_req);
  */
 int mmc_wait_for_cmd(struct mmc_host *host, struct mmc_command *cmd, int retries)
 {
-	struct mmc_request mrq = {0};
+	struct mmc_request mrq = {NULL};
 
 	WARN_ON(!host->claimed);
 
diff --git a/drivers/mmc/core/mmc_ops.c b/drivers/mmc/core/mmc_ops.c
index 845ce7c..8f1d372 100644
--- a/drivers/mmc/core/mmc_ops.c
+++ b/drivers/mmc/core/mmc_ops.c
@@ -233,7 +233,7 @@ static int
 mmc_send_cxd_data(struct mmc_card *card, struct mmc_host *host,
 		u32 opcode, void *buf, unsigned len)
 {
-	struct mmc_request mrq = {0};
+	struct mmc_request mrq = {NULL};
 	struct mmc_command cmd = {0};
 	struct mmc_data data = {0};
 	struct scatterlist sg;
@@ -454,7 +454,7 @@ static int
 mmc_send_bus_test(struct mmc_card *card, struct mmc_host *host, u8 opcode,
 		  u8 len)
 {
-	struct mmc_request mrq = {0};
+	struct mmc_request mrq = {NULL};
 	struct mmc_command cmd = {0};
 	struct mmc_data data = {0};
 	struct scatterlist sg;
diff --git a/drivers/mmc/core/sdio_ops.c b/drivers/mmc/core/sdio_ops.c
index f087d87..4addbe9 100644
--- a/drivers/mmc/core/sdio_ops.c
+++ b/drivers/mmc/core/sdio_ops.c
@@ -121,7 +121,7 @@ int mmc_io_rw_direct(struct mmc_card *card, int write, unsigned fn,
 int mmc_io_rw_extended(struct mmc_card *card, int write, unsigned fn,
 	unsigned addr, int incr_addr, u8 *buf, unsigned blocks, unsigned blksz)
 {
-	struct mmc_request mrq = {0};
+	struct mmc_request mrq = {NULL};
 	struct mmc_command cmd = {0};
 	struct mmc_data data = {0};
 	struct scatterlist sg;
-- 
1.7.1

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[Chris Ball]

Hi,

[Adding linux-sparse@ to CC]

On Tue, Aug 23 2011, Venkatraman S wrote:
> Fix the sparse warning output
> "warning: Using plain integer as NULL pointer"
>
> Signed-off-by: Venkatraman S <svenkatr@ti.com>
> ---
>  drivers/mmc/card/block.c    |    4 ++--
>  drivers/mmc/core/core.c     |    2 +-
>  drivers/mmc/core/mmc_ops.c  |    4 ++--
>  drivers/mmc/core/sdio_ops.c |    2 +-
>  4 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 1ff5486..e702c61 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -291,7 +291,7 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
>  	struct mmc_card *card;
>  	struct mmc_command cmd = {0};
>  	struct mmc_data data = {0};
> -	struct mmc_request mrq = {0};
> +	struct mmc_request mrq = {NULL};
>  	struct scatterlist sg;
>  	int err;
> [...]

The sparse warning is mistaken.  Or I'm mistaken.  But I suspect it's
the sparse warning.

The {0} syntax is covered by:

   [6.7.8.21] If there are fewer initializers in a brace-enclosed list
   than there are elements or members of an aggregate, or fewer
   characters in a string literal used to initialize an array of known
   size than there are elements in the array, the remainder of the
   aggregate shall be initialized implicitly the same as objects that
   have static storage duration.

So we're not assigning 0 to a pointer, or whatever sparse thinks we're
doing -- we're initializing every member of the struct with 0, which is
a good and safe way to initialize it.

Sparse folks, any comment?

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[Sam Ravnborg]

On Tue, Aug 23, 2011 at 12:31:55PM -0400, Chris Ball wrote:
> Hi,
> 
> [Adding linux-sparse@ to CC]
> 
> On Tue, Aug 23 2011, Venkatraman S wrote:
> > Fix the sparse warning output
> > "warning: Using plain integer as NULL pointer"
> >
> > Signed-off-by: Venkatraman S <svenkatr@ti.com>
> > ---
> >  drivers/mmc/card/block.c    |    4 ++--
> >  drivers/mmc/core/core.c     |    2 +-
> >  drivers/mmc/core/mmc_ops.c  |    4 ++--
> >  drivers/mmc/core/sdio_ops.c |    2 +-
> >  4 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> > index 1ff5486..e702c61 100644
> > --- a/drivers/mmc/card/block.c
> > +++ b/drivers/mmc/card/block.c
> > @@ -291,7 +291,7 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
> >  	struct mmc_card *card;
> >  	struct mmc_command cmd = {0};
> >  	struct mmc_data data = {0};
> > -	struct mmc_request mrq = {0};
> > +	struct mmc_request mrq = {NULL};
> >  	struct scatterlist sg;
> >  	int err;
> > [...]
> 
> The sparse warning is mistaken.  Or I'm mistaken.  But I suspect it's
> the sparse warning.
> 
> The {0} syntax is covered by:
> 
>    [6.7.8.21] If there are fewer initializers in a brace-enclosed list
>    than there are elements or members of an aggregate, or fewer
>    characters in a string literal used to initialize an array of known
>    size than there are elements in the array, the remainder of the
>    aggregate shall be initialized implicitly the same as objects that
>    have static storage duration.
> 
> So we're not assigning 0 to a pointer, or whatever sparse thinks we're
> doing -- we're initializing every member of the struct with 0, which is
> a good and safe way to initialize it.
> 
> Sparse folks, any comment?

The struct looks like this:
struct mmc_request {
        struct mmc_command      *sbc;           /* SET_BLOCK_COUNT for multiblock */
        struct mmc_command      *cmd;
        struct mmc_data         *data;
        struct mmc_command      *stop;

        struct completion       completion;
        void                    (*done)(struct mmc_request *);/* completion function */
};


So you assing '0' to sbc - which is a pointer.
So sparse warning is correct.

	Sam

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[Chris Ball]

Hi,

On Tue, Aug 23 2011, Sam Ravnborg wrote:
>> So we're not assigning 0 to a pointer, or whatever sparse thinks we're
>> doing -- we're initializing every member of the struct with 0, which is
>> a good and safe way to initialize it.
>> 
>> Sparse folks, any comment?
>
> The struct looks like this:
> struct mmc_request {
>         struct mmc_command      *sbc;           /* SET_BLOCK_COUNT for multiblock */
>         struct mmc_command      *cmd;
>         struct mmc_data         *data;
>         struct mmc_command      *stop;
>
>         struct completion       completion;
>         void (*done)(struct mmc_request *);/* completion function */
> };
>
>
> So you assing '0' to sbc - which is a pointer.
> So sparse warning is correct.

Oops, thanks Sam, I should have realized this was the complaint --
sorry for wasting your time.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[J Freyensee]

On 08/23/2011 09:56 AM, Sam Ravnborg wrote:
> On Tue, Aug 23, 2011 at 12:31:55PM -0400, Chris Ball wrote:
>> Hi,
>>
>> [Adding linux-sparse@ to CC]
>>
>> On Tue, Aug 23 2011, Venkatraman S wrote:
>>> Fix the sparse warning output
>>> "warning: Using plain integer as NULL pointer"
>>>
>>> Signed-off-by: Venkatraman S<svenkatr@ti.com>
>>> ---
>>>   drivers/mmc/card/block.c    |    4 ++--
>>>   drivers/mmc/core/core.c     |    2 +-
>>>   drivers/mmc/core/mmc_ops.c  |    4 ++--
>>>   drivers/mmc/core/sdio_ops.c |    2 +-
>>>   4 files changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>> index 1ff5486..e702c61 100644
>>> --- a/drivers/mmc/card/block.c
>>> +++ b/drivers/mmc/card/block.c
>>> @@ -291,7 +291,7 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
>>>   	struct mmc_card *card;
>>>   	struct mmc_command cmd = {0};
>>>   	struct mmc_data data = {0};
>>> -	struct mmc_request mrq = {0};
>>> +	struct mmc_request mrq = {NULL};
>>>   	struct scatterlist sg;
>>>   	int err;
>>> [...]
>>
>> The sparse warning is mistaken.  Or I'm mistaken.  But I suspect it's
>> the sparse warning.
>>

Okay, stupid question times:

 >>> -	struct mmc_request mrq = {0};

this is 'struct mmc_request mrq' and not 'struct mmc_request *mrq', right?

Then I think the sparse warning is correct.  From my experience, sparse 
is very clear: if any variable is defined as a pointer, you use NULL to 
reset the pointer instead of '0', and if any variable is a normal 
variable, you use '0' to reset the variable. I don't think sparse is 
smart enough to look at the underlying variables, which in this case, 
mrq contains members that are pointers to something.

in this case, the defined variable is not a pointer, rather a variable 
named mrq that is of type 'struct mmc_request'.  Therefore, 0 is correct 
to use and NULL is incorrect.

If you want it to be sparse-correct as well as human-readable correct, 
you probably should write the code that explicitly shows assigning each 
pointer member of mrq to NULL, ie:

struct mmc_request mrq;
mrq.sbc = NULL;
mrq.cmd = NULL;
mrq.data = NULL;
.
.
etc.



>> The {0} syntax is covered by:
>>
>>     [6.7.8.21] If there are fewer initializers in a brace-enclosed list
>>     than there are elements or members of an aggregate, or fewer
>>     characters in a string literal used to initialize an array of known
>>     size than there are elements in the array, the remainder of the
>>     aggregate shall be initialized implicitly the same as objects that
>>     have static storage duration.
>>
>> So we're not assigning 0 to a pointer, or whatever sparse thinks we're
>> doing -- we're initializing every member of the struct with 0, which is
>> a good and safe way to initialize it.
>>
>> Sparse folks, any comment?
>
> The struct looks like this:
> struct mmc_request {
>          struct mmc_command      *sbc;           /* SET_BLOCK_COUNT for multiblock */
>          struct mmc_command      *cmd;
>          struct mmc_data         *data;
>          struct mmc_command      *stop;
>
>          struct completion       completion;
>          void                    (*done)(struct mmc_request *);/* completion function */
> };
>
>
> So you assing '0' to sbc - which is a pointer.
> So sparse warning is correct.
>
> 	Sam
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
J (James/Jay) Freyensee
Storage Technology Group
Intel Corporation

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[S, Venkatraman]

On Wed, Aug 24, 2011 at 4:10 AM, J Freyensee
<james_p_freyensee@linux.intel.com> wrote:
> On 08/23/2011 09:56 AM, Sam Ravnborg wrote:
>>
>> On Tue, Aug 23, 2011 at 12:31:55PM -0400, Chris Ball wrote:
>>>
>>> Hi,
>>>
>>> [Adding linux-sparse@ to CC]
>>>
>>> On Tue, Aug 23 2011, Venkatraman S wrote:
>>>>
>>>> Fix the sparse warning output
>>>> "warning: Using plain integer as NULL pointer"
>>>>
>>>> Signed-off-by: Venkatraman S<svenkatr@ti.com>
>>>> ---
>>>>  drivers/mmc/card/block.c    |    4 ++--
>>>>  drivers/mmc/core/core.c     |    2 +-
>>>>  drivers/mmc/core/mmc_ops.c  |    4 ++--
>>>>  drivers/mmc/core/sdio_ops.c |    2 +-
>>>>  4 files changed, 6 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
>>>> index 1ff5486..e702c61 100644
>>>> --- a/drivers/mmc/card/block.c
>>>> +++ b/drivers/mmc/card/block.c
>>>> @@ -291,7 +291,7 @@ static int mmc_blk_ioctl_cmd(struct block_device
>>>> *bdev,
>>>>        struct mmc_card *card;
>>>>        struct mmc_command cmd = {0};
>>>>        struct mmc_data data = {0};
>>>> -       struct mmc_request mrq = {0};
>>>> +       struct mmc_request mrq = {NULL};
>>>>        struct scatterlist sg;
>>>>        int err;
>>>> [...]
>>>
>>> The sparse warning is mistaken.  Or I'm mistaken.  But I suspect it's
>>> the sparse warning.
>>>
>
> Okay, stupid question times:
>
>>>> -   struct mmc_request mrq = {0};
>
> this is 'struct mmc_request mrq' and not 'struct mmc_request *mrq', right?
>
> Then I think the sparse warning is correct.  From my experience, sparse is
> very clear: if any variable is defined as a pointer, you use NULL to reset
> the pointer instead of '0', and if any variable is a normal variable, you
> use '0' to reset the variable. I don't think sparse is smart enough to look
> at the underlying variables, which in this case, mrq contains members that
> are pointers to something.
>
> in this case, the defined variable is not a pointer, rather a variable named
> mrq that is of type 'struct mmc_request'.  Therefore, 0 is correct to use
> and NULL is incorrect.

Please tolerate my misunderstanding here - but aren't the above two statements
contradictory ?
My understanding was the '0' or NULL would be assigned to the first member of
struct mmc_request, which happens to be a pointer. Hence NULL is more correct
than '0', right ?

The type of the structure variable is irrelevant here. i.e.
struct mmc_request mrq = {0};
and
struct mmc_request *mrq = {0};

would generate the same warning, isn't it ?

>
> If you want it to be sparse-correct as well as human-readable correct, you
> probably should write the code that explicitly shows assigning each pointer
> member of mrq to NULL, ie:
>
> struct mmc_request mrq;
> mrq.sbc = NULL;
> mrq.cmd = NULL;
> mrq.data = NULL;
> .
> .
> etc.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[Josh Triplett]

On Tue, Aug 23, 2011 at 12:31:55PM -0400, Chris Ball wrote:
> Hi,
> 
> [Adding linux-sparse@ to CC]
> 
> On Tue, Aug 23 2011, Venkatraman S wrote:
> > Fix the sparse warning output
> > "warning: Using plain integer as NULL pointer"
> >
> > Signed-off-by: Venkatraman S <svenkatr@ti.com>
> > ---
> >  drivers/mmc/card/block.c    |    4 ++--
> >  drivers/mmc/core/core.c     |    2 +-
> >  drivers/mmc/core/mmc_ops.c  |    4 ++--
> >  drivers/mmc/core/sdio_ops.c |    2 +-
> >  4 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> > index 1ff5486..e702c61 100644
> > --- a/drivers/mmc/card/block.c
> > +++ b/drivers/mmc/card/block.c
> > @@ -291,7 +291,7 @@ static int mmc_blk_ioctl_cmd(struct block_device *bdev,
> >  	struct mmc_card *card;
> >  	struct mmc_command cmd = {0};
> >  	struct mmc_data data = {0};
> > -	struct mmc_request mrq = {0};
> > +	struct mmc_request mrq = {NULL};
> >  	struct scatterlist sg;
> >  	int err;
> > [...]
> 
> The sparse warning is mistaken.  Or I'm mistaken.  But I suspect it's
> the sparse warning.
> 
> The {0} syntax is covered by:
> 
>    [6.7.8.21] If there are fewer initializers in a brace-enclosed list
>    than there are elements or members of an aggregate, or fewer
>    characters in a string literal used to initialize an array of known
>    size than there are elements in the array, the remainder of the
>    aggregate shall be initialized implicitly the same as objects that
>    have static storage duration.
> 
> So we're not assigning 0 to a pointer, or whatever sparse thinks we're
> doing -- we're initializing every member of the struct with 0, which is
> a good and safe way to initialize it.
> 
> Sparse folks, any comment?

Notice that it says "the remainder of the aggregate".  The first field
still gets initialized with the 0 you supplied, and the first field of
struct mmc_request has a pointer type.

- Josh Triplett

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[Ben Pfaff]

Josh Triplett <josh@joshtriplett.org> writes:

> On Tue, Aug 23, 2011 at 12:31:55PM -0400, Chris Ball wrote:
>> On Tue, Aug 23 2011, Venkatraman S wrote:
>> > -	struct mmc_request mrq = {0};
>> > +	struct mmc_request mrq = {NULL};
>> 
>> The sparse warning is mistaken.  Or I'm mistaken.  But I suspect it's
>> the sparse warning.
>
> Notice that it says "the remainder of the aggregate".  The first field
> still gets initialized with the 0 you supplied, and the first field of
> struct mmc_request has a pointer type.

That's an understandable position, but I think it would also be
reasonable for sparse to special case using {0} as an
initializer.  {0} is a valid initializer for every type and so
it's sometimes used as an initializer for a local variable to get
the same effect that one would have for a static variable without
specifying an initializer.
-- 
Ben Pfaff 
http://benpfaff.org

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[Josh Triplett]

On Tue, Aug 23, 2011 at 11:04:08AM -0700, Ben Pfaff wrote:
> Josh Triplett <josh@joshtriplett.org> writes:
> 
> > On Tue, Aug 23, 2011 at 12:31:55PM -0400, Chris Ball wrote:
> >> On Tue, Aug 23 2011, Venkatraman S wrote:
> >> > -	struct mmc_request mrq = {0};
> >> > +	struct mmc_request mrq = {NULL};
> >> 
> >> The sparse warning is mistaken.  Or I'm mistaken.  But I suspect it's
> >> the sparse warning.
> >
> > Notice that it says "the remainder of the aggregate".  The first field
> > still gets initialized with the 0 you supplied, and the first field of
> > struct mmc_request has a pointer type.
> 
> That's an understandable position, but I think it would also be
> reasonable for sparse to special case using {0} as an
> initializer.  {0} is a valid initializer for every type and so
> it's sometimes used as an initializer for a local variable to get
> the same effect that one would have for a static variable without
> specifying an initializer.

{} produces the same effect, as far as I know.

- Josh Triplett

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[Chris Ball]

Hi,

On Tue, Aug 23 2011, Josh Triplett wrote:
> {} produces the same effect, as far as I know.

Yeah.  I prefer {0}, because {} is a gcc-ism (the ANSI grammar demands
initializer-lists be non-empty) and is less readable for people who
haven't seen the idiom before and are wondering what's going on.

I'm still a little confused -- the {0} or memset(0, struct ..);
formations are used often in the kernel, even with pointers involved.
Is the warning (Wnon_pointer_null) run against the kernel by default,
or did Venkatraman add it manually?  If default, is it catching bugs?

Thanks,

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[Sam Ravnborg]

On Tue, Aug 23, 2011 at 02:28:42PM -0400, Chris Ball wrote:
> Hi,
> 
> On Tue, Aug 23 2011, Josh Triplett wrote:
> > {} produces the same effect, as far as I know.
> 
> Yeah.  I prefer {0}, because {} is a gcc-ism (the ANSI grammar demands
> initializer-lists be non-empty) and is less readable for people who
> haven't seen the idiom before and are wondering what's going on.
> 
> I'm still a little confused -- the {0} or memset(0, struct ..);
> formations are used often in the kernel, even with pointers involved.
> Is the warning (Wnon_pointer_null) run against the kernel by default,
> or did Venkatraman add it manually?  If default, is it catching bugs?

It is default enabled - and trigger a lot of warnings.
But the kernel idiom is to use NULL for a null pointer - and not to use 0.

Which is why the warning was introduced.

	Sam

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[Sam Ravnborg]

> 
> I'm still a little confused -- the {0} or memset(0, struct ..);
> formations are used often in the kernel, even with pointers involved.
> Is the warning (Wnon_pointer_null) run against the kernel by default,
> or did Venkatraman add it manually?  If default, is it catching bugs?

Did a quick "allnoconfig" build. With this minimal build the warning only triggered three times.
So a lot of these has been fixed already.

	Sam

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[S, Venkatraman]

On Wed, Aug 24, 2011 at 2:08 AM, Sam Ravnborg <sam@ravnborg.org> wrote:
>>
>> I'm still a little confused -- the {0} or memset(0, struct ..);
>> formations are used often in the kernel, even with pointers involved.
>> Is the warning (Wnon_pointer_null) run against the kernel by default,
>> or did Venkatraman add it manually?  If default, is it catching bugs?
>
> Did a quick "allnoconfig" build. With this minimal build the warning only triggered three times.
> So a lot of these has been fixed already.
>
>        Sam
>
I did my typical omap build on mainline and mmc-next and see that the warnings
have increased, and not fixed.
I will post upated patch series.

Re: [PATCH 2/2] mmc: fix integer assignments to pointer

[Ben Pfaff]

Josh Triplett <josh@joshtriplett.org> writes:

> On Tue, Aug 23, 2011 at 11:04:08AM -0700, Ben Pfaff wrote:
>> Josh Triplett <josh@joshtriplett.org> writes:
>> 
>> > On Tue, Aug 23, 2011 at 12:31:55PM -0400, Chris Ball wrote:
>> >> On Tue, Aug 23 2011, Venkatraman S wrote:
>> >> > -	struct mmc_request mrq = {0};
>> >> > +	struct mmc_request mrq = {NULL};
>> >> 
>> >> The sparse warning is mistaken.  Or I'm mistaken.  But I suspect it's
>> >> the sparse warning.
>> >
>> > Notice that it says "the remainder of the aggregate".  The first field
>> > still gets initialized with the 0 you supplied, and the first field of
>> > struct mmc_request has a pointer type.
>> 
>> That's an understandable position, but I think it would also be
>> reasonable for sparse to special case using {0} as an
>> initializer.  {0} is a valid initializer for every type and so
>> it's sometimes used as an initializer for a local variable to get
>> the same effect that one would have for a static variable without
>> specifying an initializer.
>
> {} produces the same effect, as far as I know.

{} is not a valid initializer for every type in the same way as
{0}, e.g.:

    blp@hardrock:~/db$ cat > tmp.c
    int x = {};
    int y = {0};
    blp@hardrock:~/db$ gcc tmp.c
    tmp.c:1:1: error: empty scalar initializer
    tmp.c:1:1: error: (near initialization for 'x')

-- 
Ben Pfaff 
http://benpfaff.org