On Wednesday, April 17, 2013 1:15:11 pm hiren panchasara wrote: > On Tue, Apr 16, 2013 at 9:10 AM, John Baldwin <j...@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 _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
