Date: Sat, 6 Jan 2001 18:34:46 -0500 From: "Bosko Milekic" <bmilekic@technokratis.com> To: <cvs-committers@FreeBSD.org> Cc: <cvs-all@FreeBSD.org> Subject: Re: cvs commit: src/sys/dev/musycc musycc.c Message-ID: <005001c07839$40f06820$25cbca18@jehovah> References: <200101062044.f06Kiex42615@freefall.freebsd.org> <20010106132943.E15744@fw.wintelcom.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Alfred wrote:
> * Bosko Milekic <bmilekic@FreeBSD.org> [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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?005001c07839$40f06820$25cbca18>