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

S, Venkatraman
2011-08-24T11:23:07Z

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