From owner-svn-src-all@FreeBSD.ORG Wed Apr 17 18:11:18 2013 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by hub.freebsd.org (Postfix) with ESMTP id 0A1CF937; Wed, 17 Apr 2013 18:11:18 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from bigwig.baldwin.cx (bigwig.baldwin.cx [IPv6:2001:470:1f11:75::1]) by mx1.freebsd.org (Postfix) with ESMTP id 4C896B0C; Wed, 17 Apr 2013 18:11:17 +0000 (UTC) Received: from jhbbsd.localnet (unknown [209.249.190.124]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 3CADBB977; Wed, 17 Apr 2013 14:11:16 -0400 (EDT) From: John Baldwin To: hiren panchasara Subject: Re: svn commit: r249461 - head/sys/dev/ips Date: Wed, 17 Apr 2013 13:19:26 -0400 User-Agent: KMail/1.13.5 (FreeBSD/8.2-CBSD-20110714-p25; KDE/4.5.5; amd64; ; ) References: <201304140242.r3E2geSq094403@svn.freebsd.org> <201304161210.09058.jhb@freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <201304171319.26560.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (bigwig.baldwin.cx); Wed, 17 Apr 2013 14:11:16 -0400 (EDT) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 17 Apr 2013 18:11:18 -0000 On Wednesday, April 17, 2013 1:15:11 pm hiren panchasara wrote: > On Tue, Apr 16, 2013 at 9:10 AM, John Baldwin wrote: > > On Saturday, April 13, 2013 10:42:40 pm Hiren Panchasara wrote: > >> Author: hiren > >> Date: Sun Apr 14 02:42:40 2013 > >> New Revision: 249461 > >> URL: http://svnweb.freebsd.org/changeset/base/249461 > >> > >> Log: > >> Fixing a clang warning indicating uninitialized variable usage. > >> > >> PR: kern/177164 > >> Approved by: sbruno (mentor) > > > > Hmm, I always thought that dmatags and maps were opaque types and not > > necessarily "known" in consumers to be pointers. (Some drivers do check tehm > > against NULL implicitly via 'if (map)' or 'if (tag)', but well-behaved drivers > > use other means (flags, etc.) to know if they are valid.) > > Hi John, > > Would this be a better fix? We do not need to do any clean up if we > fail to create the tag: > > Index: ips.c > =================================================================== > --- ips.c (revision 249588) > +++ ips.c (working copy) > @@ -578,7 +578,7 @@ > { > int error; > bus_dma_tag_t dmatag; > - bus_dmamap_t dmamap = NULL; > + bus_dmamap_t dmamap; > if (bus_dma_tag_create( /* parent */ sc->adapter_dmatag, > /* alignemnt */ 1, > /* boundary */ 0, > @@ -595,7 +595,7 @@ > &dmatag) != 0) { > device_printf(sc->dev, "can't alloc dma tag for > statue queue\n"); > error = ENOMEM; > - goto exit; > + return error; > } > if(bus_dmamem_alloc(dmatag, (void *)&(sc->copper_queue), > BUS_DMA_NOWAIT, &dmamap)){ That would be fine. I would actually prefer though that this only call bus_dmamem_free() if the alloc succeeds, so in addition I would make the call to bus_dmammem_free() conditional on sc->copper_queue != NULL. Thanks. -- John Baldwin