[PATCH 7/7] Fix uninitialised variable warning in mei-amt-version.c

[PATCH 7/7] Fix uninitialised variable warning in mei-amt-version.c

[David Howells]

Fix the following warning:

	Documentation/misc-devices/mei/mei-amt-version.c: In function 'main':
	Documentation/misc-devices/mei/mei-amt-version.c:103:5: warning: 'acmd.fd' is used uninitialized in this function [-Wuninitialized]
	  if (cl->fd != -1)
	     ^
	Documentation/misc-devices/mei/mei-amt-version.c:443:21: note: 'acmd.fd' was declared here
	  struct amt_host_if acmd;
			     ^

The call chain:

	-->main()
	   -->amt_host_if_init()
	      -->mei_init()
	         -->mei_deinit()

results in this on the first time through because main()::acmd has not yet
been initialised.

To fix this, acmd.fd needs to be initialised to -1.

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Tomas Winkler <tomas.winkler@intel.com>
cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---

 Documentation/misc-devices/mei/mei-amt-version.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/misc-devices/mei/mei-amt-version.c b/Documentation/misc-devices/mei/mei-amt-version.c
index 49e4f770864a..7027a8da0ba0 100644
--- a/Documentation/misc-devices/mei/mei-amt-version.c
+++ b/Documentation/misc-devices/mei/mei-amt-version.c
@@ -440,7 +440,7 @@ out:
 int main(int argc, char **argv)
 {
 	struct amt_code_versions ver;
-	struct amt_host_if acmd;
+	struct amt_host_if acmd = { .mei_cl.fd = -1 };
 	unsigned int i;
 	uint32_t status;
 	int ret;

Re: [PATCH 7/7] Fix uninitialised variable warning in mei-amt-version.c

[Paul Bolle]

On Fri, 2014-01-03 at 16:08 +0000, David Howells wrote:
> Fix the following warning:
> 
> 	Documentation/misc-devices/mei/mei-amt-version.c: In function 'main':
> 	Documentation/misc-devices/mei/mei-amt-version.c:103:5: warning: 'acmd.fd' is used uninitialized in this function [-Wuninitialized]
> 	  if (cl->fd != -1)
> 	     ^
> 	Documentation/misc-devices/mei/mei-amt-version.c:443:21: note: 'acmd.fd' was declared here
> 	  struct amt_host_if acmd;
> 			     ^
> 
> The call chain:
> 
> 	-->main()
> 	   -->amt_host_if_init()
> 	      -->mei_init()
> 	         -->mei_deinit()
> 
> results in this on the first time through because main()::acmd has not yet
> been initialised.
> 
> To fix this, acmd.fd needs to be initialised to -1.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> cc: Tomas Winkler <tomas.winkler@intel.com>
> cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

I've been using a local patch now for quite some time. I currently use
it on v3.12.6. I haven't yet bothered to submit it. My (draft) commit
summary reads:

"Building mei-amt-version.o triggers a GCC warning:
    Documentation/misc-devices/mei/mei-amt-version.c: In function 'main':
    Documentation/misc-devices/mei/mei-amt-version.c:103:5: warning: 'acmd.fd' is used uninitialized in this function [-Wuninitialized]
      if (cl->fd != -1)
         ^
    Documentation/misc-devices/mei/mei-amt-version.c:443:21: note: 'acmd.fd' was declared here
      struct amt_host_if acmd;
                         ^

GCC is correct. See, the call chain that GCC detects must be
    main()
        amt_host_if_init()
            mei_init()
                mei_deinit()

But when we enter mei_deinit() struct amt_host_if acmd is still
unitialized. That makes the test for (effectively) amt_host_if->mei_cl->fd
bogus.

But it turns out that call of mei_deinit() isn't needed at all. All of
the members of mei_cl will be set later in mei_init() and none will be
used before they are set. So we can simply drop this call of
mei_deinit()."

Is that analysis (still) correct?


Paul Bolle

>  Documentation/misc-devices/mei/mei-amt-version.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/misc-devices/mei/mei-amt-version.c b/Documentation/misc-devices/mei/mei-amt-version.c
> index 49e4f770864a..7027a8da0ba0 100644
> --- a/Documentation/misc-devices/mei/mei-amt-version.c
> +++ b/Documentation/misc-devices/mei/mei-amt-version.c
> @@ -440,7 +440,7 @@ out:
>  int main(int argc, char **argv)
>  {
>  	struct amt_code_versions ver;
> -	struct amt_host_if acmd;
> +	struct amt_host_if acmd = { .mei_cl.fd = -1 };
>  	unsigned int i;
>  	uint32_t status;
>  	int ret;
> 

Re: [PATCH 7/7] Fix uninitialised variable warning in mei-amt-version.c

[David Howells]

Paul Bolle <pebolle@tiscali.nl> wrote:

> But it turns out that call of mei_deinit() isn't needed at all. All of
> the members of mei_cl will be set later in mei_init() and none will be
> used before they are set. So we can simply drop this call of
> mei_deinit()."
> 
> Is that analysis (still) correct?

It looks true as the code stands.  Can you send me your patch?

David

[PATCH] mei: mei-amt-version: remove unneeded call of mei_deinit()

[Paul Bolle]

On Fri, 2014-01-03 at 16:34 +0000, David Howells wrote:
> Paul Bolle <pebolle@tiscali.nl> wrote:
> > But it turns out that call of mei_deinit() isn't needed at all. All of
> > the members of mei_cl will be set later in mei_init() and none will be
> > used before they are set. So we can simply drop this call of
> > mei_deinit()."
> > 
> > Is that analysis (still) correct?
> 
> It looks true as the code stands.  Can you send me your patch?

Sure. Below the scissors (hope I've typed those correctly).


Paul Bolle
-------->8------- 
From: Paul Bolle <pebolle@tiscali.nl>

Building mei-amt-version.o triggers a GCC warning:
    Documentation/misc-devices/mei/mei-amt-version.c: In function 'main':
    Documentation/misc-devices/mei/mei-amt-version.c:103:5: warning: 'acmd.fd' is used uninitialized in this function [-Wuninitialized]
      if (cl->fd != -1)
         ^
    Documentation/misc-devices/mei/mei-amt-version.c:443:21: note: 'acmd.fd' was declared here
      struct amt_host_if acmd;
                         ^

GCC is correct. See, the call chain that GCC detects must be
    main()
        amt_host_if_init()
            mei_init()
                mei_deinit()

But when we enter mei_deinit() struct amt_host_if acmd is still
unitialized. That makes the test for (effectively) amt_host_if->mei_cl->fd
bogus.

But it turns out that call of mei_deinit() isn't needed at all. All of
the members of mei_cl will be set later in mei_init() and none will be
used before they are set. So we can simply drop this call of
mei_deinit().

Signed-off-by: Paul Bolle <pebolle@tiscali.nl>
---
 Documentation/misc-devices/mei/mei-amt-version.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/misc-devices/mei/mei-amt-version.c b/Documentation/misc-devices/mei/mei-amt-version.c
index 49e4f77..57d0d87 100644
--- a/Documentation/misc-devices/mei/mei-amt-version.c
+++ b/Documentation/misc-devices/mei/mei-amt-version.c
@@ -115,8 +115,6 @@ static bool mei_init(struct mei *me, const uuid_le *guid,
 	struct mei_client *cl;
 	struct mei_connect_client_data data;
 
-	mei_deinit(me);
-
 	me->verbose = verbose;
 
 	me->fd = open("/dev/mei", O_RDWR);
-- 
1.8.4.2

Re: [PATCH] mei: mei-amt-version: remove unneeded call of mei_deinit()

[David Howells]

Paul Bolle <pebolle@tiscali.nl> wrote:

> From: Paul Bolle <pebolle@tiscali.nl>
> 
> Building mei-amt-version.o triggers a GCC warning:
>     Documentation/misc-devices/mei/mei-amt-version.c: In function 'main':
>     Documentation/misc-devices/mei/mei-amt-version.c:103:5: warning: 'acmd.fd' is used uninitialized in this function [-Wuninitialized]
>       if (cl->fd != -1)
>          ^
>     Documentation/misc-devices/mei/mei-amt-version.c:443:21: note: 'acmd.fd' was declared here
>       struct amt_host_if acmd;
>                          ^
> 
> GCC is correct. See, the call chain that GCC detects must be
>     main()
>         amt_host_if_init()
>             mei_init()
>                 mei_deinit()
> 
> But when we enter mei_deinit() struct amt_host_if acmd is still
> unitialized. That makes the test for (effectively) amt_host_if->mei_cl->fd
> bogus.
> 
> But it turns out that call of mei_deinit() isn't needed at all. All of
> the members of mei_cl will be set later in mei_init() and none will be
> used before they are set. So we can simply drop this call of
> mei_deinit().
> 
> Signed-off-by: Paul Bolle <pebolle@tiscali.nl>

Builds without warning for me.

Acked-by: David Howells <dhowells@redhat.com>

RE: [PATCH 7/7] Fix uninitialised variable warning in mei-amt-version.c

[Winkler, Tomas]

> -----Original Message-----
> From: David Howells [mailto:dhowells@redhat.com]
> Sent: Friday, January 03, 2014 18:34
> To: Paul Bolle
> Cc: dhowells@redhat.com; akpm@linux-foundation.org; Greg Kroah-Hartman;
> Winkler, Tomas; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 7/7] Fix uninitialised variable warning in mei-amt-version.c
> 
> Paul Bolle <pebolle@tiscali.nl> wrote:
> 
> > But it turns out that call of mei_deinit() isn't needed at all. All of
> > the members of mei_cl will be set later in mei_init() and none will be
> > used before they are set. So we can simply drop this call of
> > mei_deinit()."
> >
> > Is that analysis (still) correct?
> 
> It looks true as the code stands.  Can you send me your patch?

Yes, the analysis is correct,  need to fix the typo in the title and but otherwise ACK.

Thanks
Tomas