Date: Wed, 17 Apr 2013 13:19:26 -0400 From: John Baldwin <jhb@freebsd.org> To: hiren panchasara <hiren@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r249461 - head/sys/dev/ips Message-ID: <201304171319.26560.jhb@freebsd.org> In-Reply-To: <CALCpEUFPrRMxpksajnjAkzjXnXj20ULxa-aNdm_Ae9LOvVGSWw@mail.gmail.com> References: <201304140242.r3E2geSq094403@svn.freebsd.org> <201304161210.09058.jhb@freebsd.org> <CALCpEUFPrRMxpksajnjAkzjXnXj20ULxa-aNdm_Ae9LOvVGSWw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday, April 17, 2013 1:15:11 pm hiren panchasara wrote: > On Tue, Apr 16, 2013 at 9:10 AM, John Baldwin <jhb@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201304171319.26560.jhb>