[PATCH] src/swapon.c: initiate p to NULL

[PATCH] src/swapon.c: initiate p to NULL

[Sun Ke]

when run make, show:
swapon.c:135:3: warning: 'p' may be used uninitialized in this function
[-Wmaybe-uninitialized]
   memcpy(p, buf, BUF_SIZE);
      ^~~~~~~~~~~~~~~~~~~~~~~~
fix it.

Signed-off-by: Sun Ke <sunke32@huawei.com>
---
 src/swapon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/swapon.c b/src/swapon.c
index afaed405..fe8949a3 100644
--- a/src/swapon.c
+++ b/src/swapon.c
@@ -41,7 +41,7 @@ int main(int argc, char **argv)
 		.sa_handler	= handle_signal,
 	};
 	enum verbs verb = TEST_SWAPON;
-	void *p;
+	void *p = NULL;
 	ssize_t sz;
 	int fd = -1;
 	int ret, c;
-- 
2.13.6

Re: [PATCH] src/swapon.c: initiate p to NULL

[Zorro Lang]

On Thu, Jan 28, 2021 at 02:50:10PM +0800, Sun Ke wrote:
> when run make, show:
> swapon.c:135:3: warning: 'p' may be used uninitialized in this function
> [-Wmaybe-uninitialized]
>    memcpy(p, buf, BUF_SIZE);
>       ^~~~~~~~~~~~~~~~~~~~~~~~
> fix it.

If you don't have a situation to prove this's a real bug, I guess the gcc might
think the 'p' can be used uninitialized at here when verb is TEST_MWRITE_BEFORE_AND_MWRITE_AFTER:

        case TEST_MWRITE_BEFORE_AND_MWRITE_AFTER:
                memcpy(p, buf, BUF_SIZE);
                break;

But from the code logic, if verb is TEST_MWRITE_BEFORE_AND_MWRITE_AFTER, the 'p'
will be initialized by:

        switch (verb) {
        case TEST_MWRITE_BEFORE_AND_MWRITE_AFTER:
        case TEST_MWRITE_BEFORE:
                p = mmap(NULL, BUF_SIZE, PROT_WRITE | PROT_READ, MAP_SHARED,


So I think this's a fake uninitialized warning (Maybe I'm wrong. But if I'm right,
better to specify that in commit log).

Anyway, I personally don't object to initialize a pointer to NULL at beginning,
and make a build warning disappear. Leave the decision to maintainer :)


Thanks,
Zorro

> 
> Signed-off-by: Sun Ke <sunke32@huawei.com>
> ---
>  src/swapon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/swapon.c b/src/swapon.c
> index afaed405..fe8949a3 100644
> --- a/src/swapon.c
> +++ b/src/swapon.c
> @@ -41,7 +41,7 @@ int main(int argc, char **argv)
>  		.sa_handler	= handle_signal,
>  	};
>  	enum verbs verb = TEST_SWAPON;
> -	void *p;
> +	void *p = NULL;
>  	ssize_t sz;
>  	int fd = -1;
>  	int ret, c;
> -- 
> 2.13.6
> 

Re: [PATCH] src/swapon.c: initiate p to NULL

[sunke (E)]

hi Zorro

在 2021/1/30 12:58, Zorro Lang 写道:
> On Thu, Jan 28, 2021 at 02:50:10PM +0800, Sun Ke wrote:
>> when run make, show:
>> swapon.c:135:3: warning: 'p' may be used uninitialized in this function
>> [-Wmaybe-uninitialized]
>>     memcpy(p, buf, BUF_SIZE);
>>        ^~~~~~~~~~~~~~~~~~~~~~~~
>> fix it.
> If you don't have a situation to prove this's a real bug, I guess the gcc might
> think the 'p' can be used uninitialized at here when verb is TEST_MWRITE_BEFORE_AND_MWRITE_AFTER:
>
>          case TEST_MWRITE_BEFORE_AND_MWRITE_AFTER:
>                  memcpy(p, buf, BUF_SIZE);
>                  break;
>
> But from the code logic, if verb is TEST_MWRITE_BEFORE_AND_MWRITE_AFTER, the 'p'
> will be initialized by:
>
>          switch (verb) {
>          case TEST_MWRITE_BEFORE_AND_MWRITE_AFTER:
>          case TEST_MWRITE_BEFORE:
>                  p = mmap(NULL, BUF_SIZE, PROT_WRITE | PROT_READ, MAP_SHARED,
>
>
> So I think this's a fake uninitialized warning (Maybe I'm wrong. But if I'm right,
> better to specify that in commit log).

Yes, you are right.

Sorry, I did not analyse the code logic carefully.

>
> Anyway, I personally don't object to initialize a pointer to NULL at beginning,
> and make a build warning disappear. Leave the decision to maintainer :)
I will improve the commit.
>
> Thanks,
> Zorro
Thanks,
Sun Ke
>
>> Signed-off-by: Sun Ke <sunke32@huawei.com>
>> ---
>>   src/swapon.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/src/swapon.c b/src/swapon.c
>> index afaed405..fe8949a3 100644
>> --- a/src/swapon.c
>> +++ b/src/swapon.c
>> @@ -41,7 +41,7 @@ int main(int argc, char **argv)
>>   		.sa_handler	= handle_signal,
>>   	};
>>   	enum verbs verb = TEST_SWAPON;
>> -	void *p;
>> +	void *p = NULL;
>>   	ssize_t sz;
>>   	int fd = -1;
>>   	int ret, c;
>> -- 
>> 2.13.6
>>
> .