[PATCH]: if (cond); foo() in firewire
Andriy Gapon
avg at icyb.net.ua
Mon Jun 22 11:40:53 UTC 2009
on 22/06/2009 07:54 Steve Kargl said the following:
> On Mon, Jun 22, 2009 at 12:04:49PM +0800, Adrian Chadd wrote:
>> 2009/6/21 Roman Divacky <rdivacky at freebsd.org>:
>>> hi
>>>
>>> is this patch correct? may I commit it?
>>>
>>> Index: ../../../dev/firewire/fwdev.c
>>> ===================================================================
>>> --- ../../../dev/firewire/fwdev.c (revision 194573)
>>> +++ ../../../dev/firewire/fwdev.c (working copy)
>>> @@ -443,7 +443,7 @@
>>> xfer->send.pay_len = uio->uio_resid;
>>> if (uio->uio_resid > 0) {
>>> if ((err = uiomove((caddr_t)&xfer->send.payload[0],
>>> - uio->uio_resid, uio)));
>>> + uio->uio_resid, uio)))
>>> goto out;
>>> }
>>>
>>>
>>> another bug found by the "useless warnings in clang" ;)
>> Is clang also evaluating all subsequent execution paths to tell you
>> what the change in program flow is? :)
>>
>> I hate to be the harbinger of evilness, but I'd at least attempt a
>> cursory glance at the code to make sure subsequent code is doing the
>> right thing. (It certainly looks like a vanilla userland transfer!)
You confuse me. It is a "vanilla userland transfer", but so?
Current code always goes to "out" label regardless if uimove succeeded or not.
I think the idea was to go "out" only if uimove failed and execute some code
between if and out-label otherwise.
> I agree with you. Nothing like side effects to screw up
> a persons clang.
You confuse me, what this has to do with side-effects?
I think that Clang is right - 'if' without "then" is suspicious because either you
have a useless/redundant 'if' statement (as in you example below - just call
side_effect(&i) without putting it under if) or you accidentally put semicolon
where you shouldn't have.
> #include <stdio.h>
> #include <stdlib.h>
>
> static int
> side_effect(int *i)
> {
> *i = 42;
> return 0;
> }
>
> int
> main(void)
> {
> int i;
> if (side_effect(&i));
> if (i == 42)
> printf("%d\n", i);
> return 0;
> }
>
--
Andriy Gapon
More information about the freebsd-current
mailing list