Bug in qeth in 2.6

Bug in qeth in 2.6

[Pete Zaitcev]

This code is obviously broken in case several cards are present,
look what happens to ``result''.

devices/s390/net/qeth.c:

static int
qeth_verify_dev(struct net_device *dev)
{
        struct qeth_card *tmp;
        int result = 0;
                                                                                
        read_lock(&list_lock);
        tmp = firstcard;
        for (; tmp && (!result); tmp = tmp->next) {
                if (atomic_read(&tmp->shutdown_phase))
                        continue;
                result = (dev == tmp->dev)?
                        QETH_VERIFY_IS_REAL_DEV:__qeth_verify_dev_vlan(dev, tmp);
        }
        read_unlock(&list_lock);
        return result;
}

Someone got seriously carried away with cleanups.
I have to say, I'm disappointed. The original qeth in 2.4
had poor C style, but it was a mistake to run amok adding
spaces. The diff between 2.4 and 2.6 is too hard to read,
even with -b. Now I have to review whole darn thing line by line.
Oh woe!

-- Pete

Re: Bug in qeth in 2.6

[Cornelia Huck]

Hi Pete,

could you please elaborate what's exactly wrong with qeth_verify_dev()?

It has the same behaviour as in 2.4, and it looks correct to us...

Best regards / Mit freundlichen Gruessen

Cornelia Huck
zLinux Developer
Tel.: +49-7031-16-4837, Mail: cohuck@de.ibm.com

Re: Bug in qeth in 2.6

[Pete Zaitcev]

On Thu, Sep 04, 2003 at 10:54:43AM +0200, Cornelia Huck wrote:
> could you please elaborate what's exactly wrong with qeth_verify_dev()?
> 
> It has the same behaviour as in 2.4, and it looks correct to us...

2.4 was like this:

	result = 0;
	for (all in list) {
		if (something)
			result = QETH_VERIFY_IS_SOMETHING;
	}

2.6 is:

	result = 0;
	for (all in list) {
		result = (something)? QETH_VERIFY_IS_SOMETHING: foo();
	}

Results are different if two cards are present on the list
and the function is asked to verify the first card.

-- Pete

Re: Bug in qeth in 2.6

[Cornelia Huck]

Hi,

sorry, I still don't see it...

> 2.4 was like this:
>
>            result = 0;
>            for (all in list) {
>                        if (something)
>                                    result = QETH_VERIFY_IS_SOMETHING;

and it stops here if something...

>            }

... and here follows a second loop over the list, looking for foo().

>
> 2.6 is:
>
>            result = 0;
>            for (all in list) {
>                        result = (something)? QETH_VERIFY_IS_SOMETHING:
foo();

and here it stops if something or foo()...

>            }

...no second loop here.

Since it's either something or foo(), this looks like the same behaviour to
me.

Best regards / Mit freundlichen Gruessen

Cornelia Huck
zLinux Developer
Tel.: +49-7031-16-4837, Mail: cohuck@de.ibm.com