Re: [PATCH 4/8] Grow: Grow_addbitmap(): Add check to quiet down static code checkers

[NeilBrown <neilb@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 1676 bytes --]

On Thu, Mar 10 2016, Jes Sorensen wrote:

> Guoqing Jiang <gqjiang@suse.com> writes:
>> On 03/09/2016 01:30 AM, Jes.Sorensen@redhat.com wrote:
>>> From: Jes Sorensen <Jes.Sorensen@redhat.com>
>>>
>>> Grow_addbitmap() is only ever called with s->bitmap_file != NULL, but
>>> not all static code checkers catch this. This adds a check to quiet
>>> down the false positive warnings.
>>>
>>> Signed-off-by: Jes Sorensen <Jes.Sorensen@redhat.com>
>>> ---
>>>   Grow.c | 9 ++++++++-
>>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Grow.c b/Grow.c
>>> index 0fa776d..c453eb6 100755
>>> --- a/Grow.c
>>> +++ b/Grow.c
>>> @@ -297,7 +297,14 @@ int Grow_addbitmap(char *devname, int fd, struct context *c, struct shape *s)
>>>   			"  between different architectures.  Consider upgrading the Linux kernel.\n");
>>>   	}
>>>   -	if (s->bitmap_file && strcmp(s->bitmap_file, "clustered") ==
>>> 0)
>>> +	/*
>>> +	 * We only ever get called if s->bitmap_file is != NULL, so this check
>>> +	 * is just here to quiet down static code checkers.
>>> +	 */
>>> +	if (!s->bitmap_file)
>>> +		return 1;
>>
>> Is it really need to make all static code checkers happy? ;-)
>> Otherwise, I would prefer remove above check.
>>
>> Anyway, I am fine with the changes.
>
> We had a check in one place, but not in the remaining places. I just
> made it more consistent.

I wonder if maybe the checker was only complaining because the code was
inconsistent.
i.e. if we just got rid of the existing test on s->bitmap_file, maybe
that would make the checker happy.
It would be interesting to experiment even if you ultimately decide to
leave the new test there.

Thanks,
NeilBrown

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 818 bytes --]