From owner-freebsd-current@FreeBSD.ORG Mon Jun 22 11:40:52 2009 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B6AA2106566C; Mon, 22 Jun 2009 11:40:52 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from citadel.icyb.net.ua (citadel.icyb.net.ua [212.40.38.140]) by mx1.freebsd.org (Postfix) with ESMTP id 5968C8FC13; Mon, 22 Jun 2009 11:40:51 +0000 (UTC) (envelope-from avg@icyb.net.ua) Received: from odyssey.starpoint.kiev.ua (alpha-e.starpoint.kiev.ua [212.40.38.101]) by citadel.icyb.net.ua (8.8.8p3/ICyb-2.3exp) with ESMTP id OAA09381; Mon, 22 Jun 2009 14:20:55 +0300 (EEST) (envelope-from avg@icyb.net.ua) Message-ID: <4A3F6917.7040806@icyb.net.ua> Date: Mon, 22 Jun 2009 14:20:55 +0300 From: Andriy Gapon User-Agent: Thunderbird 2.0.0.21 (X11/20090406) MIME-Version: 1.0 To: Steve Kargl , Adrian Chadd References: <20090621082022.GA88526@freebsd.org> <20090622045428.GA18123@troutmask.apl.washington.edu> In-Reply-To: <20090622045428.GA18123@troutmask.apl.washington.edu> X-Enigmail-Version: 0.95.7 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Roman Divacky , current@freebsd.org Subject: Re: [PATCH]: if (cond); foo() in firewire X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Jun 2009 11:40:53 -0000 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 : >>> 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 > #include > > 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