From owner-cvs-all Sat Jan 6 15:33:38 2001 From owner-cvs-all@FreeBSD.ORG Sat Jan 6 15:33:32 2001 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from VL-MS-MR001.sc1.videotron.ca (relais.videotron.ca [24.201.245.36]) by hub.freebsd.org (Postfix) with ESMTP id D851937B400; Sat, 6 Jan 2001 15:33:31 -0800 (PST) Received: from jehovah ([24.202.203.37]) by VL-MS-MR001.sc1.videotron.ca (Netscape Messaging Server 4.15) with SMTP id G6RLFV02.QTS; Sat, 6 Jan 2001 18:33:31 -0500 Message-ID: <005001c07839$40f06820$25cbca18@jehovah> From: "Bosko Milekic" To: Cc: References: <200101062044.f06Kiex42615@freefall.freebsd.org> <20010106132943.E15744@fw.wintelcom.net> Subject: Re: cvs commit: src/sys/dev/musycc musycc.c Date: Sat, 6 Jan 2001 18:34:46 -0500 MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Priority: 3 X-MSMail-Priority: Normal X-Mailer: Microsoft Outlook Express 5.00.2919.6700 X-MimeOLE: Produced By Microsoft MimeOLE V5.00.2919.6700 Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk X-Loop: FreeBSD.ORG Alfred wrote: > * Bosko Milekic [010106 12:44] wrote: > > bmilekic 2001/01/06 12:44:40 PST > > > > Modified files: > > sys/dev/musycc musycc.c > > Log: > > Make sure musycc driver deals with the possibility of any type of mbuf > > allocation not succeeding. > > > > In this case, make sure the driver doesn't leak any memory by freeing all > > necessary buffers; make sure to loop and free all the previously allocated > > mbufs in this routine. > > > > Reviewed by: alfred > > Grr, This isn't how it was supposed to be done. Would you mind terribly > if I cleaned it up to what I intended it to be: > > > Index: musycc.c > =================================================================== > RCS file: /home/ncvs/src/sys/dev/musycc/musycc.c,v > retrieving revision 1.14 > diff -u -c -r1.14 musycc.c > cvs server: conflicting specifications of output style > *** musycc.c 2001/01/06 20:44:39 1.14 > --- musycc.c 2001/01/06 21:28:57 > *************** > *** 1207,1220 **** > if (m == NULL) > goto errfree; > MCLGET(m, M_TRYWAIT); > ! if ((m->m_flags M_EXT) == 0) { > ! /* We've waited mbuf_wait and still got nothing. > ! We're calling with M_TRYWAIT anyway - a little > ! defensive programming costs us very little - if > ! anything at all in the case of error. */ > ! m_free(m); > goto errfree; > - } > sc->mdr[ch][i].m = m; > sc->mdr[ch][i].data = vtophys(m->m_data); > sc->mdr[ch][i].status = 1600; /* MTU */ > --- 1207,1214 ---- > if (m == NULL) > goto errfree; > MCLGET(m, M_TRYWAIT); > ! if ((m->m_flags M_EXT) == 0) > goto errfree; > sc->mdr[ch][i].m = m; > sc->mdr[ch][i].data = vtophys(m->m_data); > sc->mdr[ch][i].status = 1600; /* MTU */ > *************** > *** 1236,1241 **** > --- 1230,1237 ---- > return (0); > > errfree: > + if (m != NULL) > + m_free(m); > while (i > 0) { > /* Don't leak all the previously allocated mbufs in this loop */ > i--; > > -- > -Alfred Perlstein - [bright@wintelcom.net|alfred@freebsd.org] > "I have the heart of a child; I keep it in a jar on my desk." I find this incredibly dumb. You're just wasting a check that need not be made. I know when I have to free m and when not to, it's dumb to have to check it again in errfree. It's Poul's call, however. To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message