* Re: [PATCH] Removes warning related to use of uninitialized
2011-01-13 2:26 [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
@ 2011-01-13 3:05 ` Greg KH
2011-01-13 4:05 ` [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2011-01-13 3:05 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Removes warning related to use of uninitialized variable i
2011-01-13 2:26 [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
2011-01-13 3:05 ` [PATCH] Removes warning related to use of uninitialized Greg KH
@ 2011-01-13 4:05 ` Sukanto Ghosh
2011-01-13 5:22 ` [PATCH] Removes warning related to use of uninitialized Dan Carpenter
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Sukanto Ghosh @ 2011-01-13 4:05 UTC (permalink / raw)
To: kernel-janitors
> 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Removes warning related to use of uninitialized
2011-01-13 2:26 [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
2011-01-13 3:05 ` [PATCH] Removes warning related to use of uninitialized Greg KH
2011-01-13 4:05 ` [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
@ 2011-01-13 5:22 ` Dan Carpenter
2011-01-13 6:49 ` [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2011-01-13 5:22 UTC (permalink / raw)
To: kernel-janitors
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);
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Removes warning related to use of uninitialized variable i
2011-01-13 2:26 [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
` (2 preceding siblings ...)
2011-01-13 5:22 ` [PATCH] Removes warning related to use of uninitialized Dan Carpenter
@ 2011-01-13 6:49 ` Sukanto Ghosh
2011-01-13 8:21 ` [PATCH] Removes warning related to use of uninitialized variable walter harms
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: Sukanto Ghosh @ 2011-01-13 6:49 UTC (permalink / raw)
To: kernel-janitors
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;
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] Removes warning related to use of uninitialized variable
2011-01-13 2:26 [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
` (3 preceding siblings ...)
2011-01-13 6:49 ` [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
@ 2011-01-13 8:21 ` walter harms
2011-01-13 18:29 ` [PATCH] Removes warning related to use of uninitialized Greg KH
2011-01-13 19:38 ` [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
6 siblings, 0 replies; 8+ messages in thread
From: walter harms @ 2011-01-13 8:21 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Removes warning related to use of uninitialized
2011-01-13 2:26 [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
` (4 preceding siblings ...)
2011-01-13 8:21 ` [PATCH] Removes warning related to use of uninitialized variable walter harms
@ 2011-01-13 18:29 ` Greg KH
2011-01-13 19:38 ` [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
6 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2011-01-13 18:29 UTC (permalink / raw)
To: kernel-janitors
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] Removes warning related to use of uninitialized variable i
2011-01-13 2:26 [PATCH] Removes warning related to use of uninitialized variable i Sukanto Ghosh
` (5 preceding siblings ...)
2011-01-13 18:29 ` [PATCH] Removes warning related to use of uninitialized Greg KH
@ 2011-01-13 19:38 ` Sukanto Ghosh
6 siblings, 0 replies; 8+ messages in thread
From: Sukanto Ghosh @ 2011-01-13 19:38 UTC (permalink / raw)
To: kernel-janitors
Just wanted to get rid of the annoying warning on gcc 4.2.2
Thanks and Regards,
Sukanto
^ permalink raw reply [flat|nested] 8+ messages in thread