[PATCH] Removes warning related to use of uninitialized variable i

[PATCH] Removes warning related to use of uninitialized variable i

[Sukanto Ghosh]

Diff against 2.6.37


Signed-off-by: Sukanto Ghosh <sukanto.cse.iitb@gmail.com>

----
diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index b25e6e4..add97dd 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1724,7 +1724,7 @@ static int serial_link_irq_chain(struct
uart_8250_port *up)

 static void serial_unlink_irq_chain(struct uart_8250_port *up)
 {
-       struct irq_info *i;
+       struct irq_info *i = NULL;
        struct hlist_node *n;
        struct hlist_head *h;

@@ -1739,7 +1739,7 @@ static void serial_unlink_irq_chain(struct
uart_8250_port *up)
        }

        BUG_ON(n = NULL);
-       BUG_ON(i->head = NULL);
+       BUG_ON(i = NULL || i->head = NULL);

        if (list_empty(i->head))
                free_irq(up->port.irq, i);

Re: [PATCH] Removes warning related to use of uninitialized

[Greg KH]

On Wed, Jan 12, 2011 at 06:26:43PM -0800, Sukanto Ghosh wrote:
> Diff against 2.6.37
> 
> 
> Signed-off-by: Sukanto Ghosh <sukanto.cse.iitb@gmail.com>
> 
> ----
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index b25e6e4..add97dd 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -1724,7 +1724,7 @@ static int serial_link_irq_chain(struct
> uart_8250_port *up)
> 
>  static void serial_unlink_irq_chain(struct uart_8250_port *up)
>  {
> -       struct irq_info *i;
> +       struct irq_info *i = NULL;
>         struct hlist_node *n;
>         struct hlist_head *h;
> 
> @@ -1739,7 +1739,7 @@ static void serial_unlink_irq_chain(struct
> uart_8250_port *up)
>         }
> 
>         BUG_ON(n = NULL);
> -       BUG_ON(i->head = NULL);
> +       BUG_ON(i = NULL || i->head = NULL);

If you just set it to NULL, how could it ever not be NULL, so why add
this check?

Also, no, this isn't correct, i is used in the hlist traversal.

thanks,

greg k-h

Re: [PATCH] Removes warning related to use of uninitialized variable i

[Sukanto Ghosh]

> If you just set it to NULL, how could it ever not be NULL, so why add
> this check?
>
Because, there is an assignment to i inside the for-each loop (hlist_for_each)
if it enters the loop.

> Also, no, this isn't correct, i is used in the hlist traversal.
>
Indeed it is being used but what if it doesn't enters the loop i.e. if
h->first is NULL
It is a different issue whether (h->first != NULL) is a precondition
to this function
but the compiler is warning because there exists a path to access of i->head
where it might be left uninitialized.

-- 
Regards,
Sukanto Ghosh

Re: [PATCH] Removes warning related to use of uninitialized

[Dan Carpenter]

On Wed, Jan 12, 2011 at 06:26:43PM -0800, Sukanto Ghosh wrote:
> Diff against 2.6.37
> 

Please specify which version of gcc you are using in your change log.

> 
> Signed-off-by: Sukanto Ghosh <sukanto.cse.iitb@gmail.com>
> 
> ----
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index b25e6e4..add97dd 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -1724,7 +1724,7 @@ static int serial_link_irq_chain(struct
> uart_8250_port *up)
> 
>  static void serial_unlink_irq_chain(struct uart_8250_port *up)
>  {
> -       struct irq_info *i;
> +       struct irq_info *i = NULL;

Use the uninitialized_var() macro instead of initializing it to NULL.

>         struct hlist_node *n;
>         struct hlist_head *h;
> 
> @@ -1739,7 +1739,7 @@ static void serial_unlink_irq_chain(struct
> uart_8250_port *up)
>         }
> 
>         BUG_ON(n = NULL);
> -       BUG_ON(i->head = NULL);
> +       BUG_ON(i = NULL || i->head = NULL);

This change won't work with the uninitialized_var() macro and is not
needed anyway (your changelog says that we're only doing this to silence
a warning).

regards,
dan carpenter

> 
>         if (list_empty(i->head))
>                 free_irq(up->port.irq, i);

Re: [PATCH] Removes warning related to use of uninitialized variable i

[Sukanto Ghosh]

The warning is given by GCC 4.2.2 (provided with eldk cross-compiler
4.2 for ppc_4xx)
Resending the patch with uninitialized_var()

Signed-off-by: Sukanto Ghosh <sukanto.cse.iitb@gmail.com>
--


diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
index b25e6e4..05c0ba3 100644
--- a/drivers/serial/8250.c
+++ b/drivers/serial/8250.c
@@ -1724,7 +1724,7 @@ static int serial_link_irq_chain(struct
uart_8250_port *up)

 static void serial_unlink_irq_chain(struct uart_8250_port *up)
 {
-       struct irq_info *i;
+       struct irq_info *uninitialized_var(i);
        struct hlist_node *n;
        struct hlist_head *h;

Re: [PATCH] Removes warning related to use of uninitialized variable

[walter harms]

Am 13.01.2011 07:49, schrieb Sukanto Ghosh:
> The warning is given by GCC 4.2.2 (provided with eldk cross-compiler
> 4.2 for ppc_4xx)
> Resending the patch with uninitialized_var()
> 
> Signed-off-by: Sukanto Ghosh <sukanto.cse.iitb@gmail.com>
> --
> 
> 
> diff --git a/drivers/serial/8250.c b/drivers/serial/8250.c
> index b25e6e4..05c0ba3 100644
> --- a/drivers/serial/8250.c
> +++ b/drivers/serial/8250.c
> @@ -1724,7 +1724,7 @@ static int serial_link_irq_chain(struct
> uart_8250_port *up)
> 
>  static void serial_unlink_irq_chain(struct uart_8250_port *up)
>  {
> -       struct irq_info *i;
> +       struct irq_info *uninitialized_var(i);
>         struct hlist_node *n;
>         struct hlist_head *h;


I have a minor issue with the variable naming, IMHO it violates
the rule of "least surprise", short names are normaly used for
integer loops like for(i=0;i<10,i++). Perhaps some kind soul
can change that into something more readable >(eg. info, node,head).

note: this comment is not directly related to the patch, just
       my 2 cents

re,
 wh

Re: [PATCH] Removes warning related to use of uninitialized

[Greg KH]

On Wed, Jan 12, 2011 at 08:05:10PM -0800, Sukanto Ghosh wrote:
> > If you just set it to NULL, how could it ever not be NULL, so why add
> > this check?
> >
> Because, there is an assignment to i inside the for-each loop (hlist_for_each)
> if it enters the loop.

Which it always will do, right?  So there isn't a problem.

> > Also, no, this isn't correct, i is used in the hlist traversal.
> >
> Indeed it is being used but what if it doesn't enters the loop i.e. if
> h->first is NULL

Can that ever happen?

> It is a different issue whether (h->first != NULL) is a precondition
> to this function

I think it is as it works fine today :)

> but the compiler is warning because there exists a path to access of i->head
> where it might be left uninitialized.

But again, that can never happen, right?

thanks,

greg k-h

Re: [PATCH] Removes warning related to use of uninitialized variable i

[Sukanto Ghosh]

Just wanted to get rid of the annoying warning on gcc 4.2.2

Thanks and Regards,
Sukanto