On Thu, 2018-02-08 at 15:39 -0600, Bjorn Helgaas wrote:
> I don't understand how this fix works. We used to check the result of
> of_irq_parse_and_map_pci() and entered the block if it was zero.
>
> Now you enter the block if it is zero or less than zero, but:
>
> static int pci_read_irq_line(...)
> {
> unsigned int virq = 0; /* unnecessarily initialized, BTW */
>
> virq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
> if (virq <= 0) {
> ...
>
> virq is unsigned, so "virq < 0" can never be true. So how does this
> change anything?
Yes it does:
So the unsigned thing is a second bug in the original patch that Alexey
isn't fixing, we need to fix it too.
However, the actual bug Alexey is fixing is that we lost the actual
value of virq. IE, without his fix, we test it for 0 but we don't
actually return it if it's positive.
So he fixes the normal case but there's still a bug in the error case,
we need to make virq signed.
Cheers,
Ben.
