[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH RFC] x86/irq: Fix maybe uninitalised issue in map_domain_pirq()

>>> On 25.01.18 at 20:14, <andrew.cooper3@xxxxxxxxxx> wrote:
> When compiling at -O3, GCC 7.2 reports:
>   irq.c: In function 'map_domain_pirq':
>   irq.c:1271:20: error: 'info' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>        pirq->arch.irq = irq;
>        ~~~~~~~~~~~~~~~^~~~~
>   irq.c:1917:18: note: 'info' was declared here
>        struct pirq *info;
>                     ^~~~
> This is a real issue, and is caused by different error style confusion in
> prepare_domain_irq_pirq().  A positive return value from radix_tree_insert()
> will take the early error path, and report success to map_domain_pirq().
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> I don't think this is the right change to make, but initialising note to NULL
> is definitely the wrong thing to do.  Thoughts?

What I'd first of all like to understand - what about the two other
call sites? They look like the same issue should arise there.
init_domain_irq_mapping() has ASSERT(err < 0) - does that
prevent it (I don't think it should)?

Assuming you mean "info" instead of "note", I agree that setting
the pointer to NULL (or actually any pointer that shouldn't be
de-referenced) would be wrong. However, I'm struggling to see
why you say "this is a real issue": radix_tree_insert() won't ever
return a positive value; it's just that the compiler can't know that.
Perhaps an option would be to (pointlessly) convert a theoretical
positive value there into some absurd -E...?


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.