Date: Mon, 22 Jun 2009 14:20:55 +0300 From: Andriy Gapon <avg@icyb.net.ua> To: Steve Kargl <sgk@troutmask.apl.washington.edu>, Adrian Chadd <adrian@freebsd.org> Cc: Roman Divacky <rdivacky@freebsd.org>, current@freebsd.org Subject: Re: [PATCH]: if (cond); foo() in firewire Message-ID: <4A3F6917.7040806@icyb.net.ua> In-Reply-To: <20090622045428.GA18123@troutmask.apl.washington.edu> References: <20090621082022.GA88526@freebsd.org> <d763ac660906212104w4bf066eatf5529779e603bd0e@mail.gmail.com> <20090622045428.GA18123@troutmask.apl.washington.edu>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4A3F6917.7040806>