Date: Sun, 30 Dec 2001 16:45:31 +0300 (MSK) From: Maxim Konovalov <maxim@macomnet.ru> To: Sheldon Hearn <sheldonh@starjuice.net> Cc: Poul-Henning Kamp <phk@FreeBSD.ORG>, <current@FreeBSD.ORG>, <chad@FreeBSD.ORG> Subject: Re: Junior Kernel Hacker Task: ccdinit stack usage. Message-ID: <20011230160821.L36208-100000@news1.macomnet.ru> In-Reply-To: <89119.1009717456@axl.seasidesoftware.co.za>
next in thread | previous in thread | raw e-mail | index | archive | help
Hello Sheldon,
On 15:04+0200, Dec 30, 2001, Sheldon Hearn wrote:
>
> [Chad David copied for last comment in message.]
>
> On Sun, 30 Dec 2001 15:28:23 +0300, Maxim Konovalov wrote:
>
> > > sys/dev/ccd/ccd.c:ccdinit() has a couple of very large items on
> > > the stack.
> > >
> > > Rewrite ccdinit() to allocate them with MALLOC(9) instead.
> >
> > tmppath is a rather big one but I can't find the second item. What
> > about this patch:
>
> I think the others are the partinfo and ccdgeom structures.
struct partinfo holds only two pointers, ccg is a pointer too.
> Note that you don't need to (and shouldn't as per style(9)) initialize
> tmppath to NULL.
Do you mean:
: Be careful to not obfuscate the code by initializing variables
: in the declarations. Use this feature only thoughtfully. DO NOT
: use function calls in initializers.
or something else?
> Also, your bzero() is unnecessary if you use the M_ZERO flag to
> MALLOC(9).
MALLOC(9) is above on for() cycle but bzero(3) is not needed even
without M_ZERO because copyinstr(9) copies the terminating NULL too.
bzero(3) was in original code so I decided to leave it.
> As an aside, what's the undocumented M_DEVBUF flag for?
man 9 malloc:
: A type is defined using the malloc_type_t typedef via the
: MALLOC_DECLARE() and MALLOC_DEFINE() macros.
Take a look at /usr/src/sys/sys/malloc.h.
What about this one:
Index: ccd.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/ccd/ccd.c,v
retrieving revision 1.95
diff -u -r1.95 ccd.c
--- ccd.c 17 Nov 2001 00:46:08 -0000 1.95
+++ ccd.c 30 Dec 2001 13:42:05 -0000
@@ -394,7 +394,7 @@
int maxsecsize;
struct partinfo dpart;
struct ccdgeom *ccg = &cs->sc_geom;
- char tmppath[MAXPATHLEN];
+ char *tmppath = NULL;
int error = 0;
#ifdef DEBUG
@@ -414,6 +414,7 @@
*/
maxsecsize = 0;
minsize = 0;
+ tmppath = malloc(MAXPATHLEN, M_DEVBUF, M_WAITOK);
for (ix = 0; ix < cs->sc_nccdisks; ix++) {
vp = cs->sc_vpp[ix];
ci = &cs->sc_cinfo[ix];
@@ -422,7 +423,6 @@
/*
* Copy in the pathname of the component.
*/
- bzero(tmppath, sizeof(tmppath)); /* sanity */
if ((error = copyinstr(cpaths[ix], tmppath,
MAXPATHLEN, &ci->ci_pathlen)) != 0) {
#ifdef DEBUG
@@ -488,6 +488,9 @@
cs->sc_size += size;
}
+ free(tmppath, M_DEVBUF);
+ tmppath = NULL;
+
/*
* Don't allow the interleave to be smaller than
* the biggest component sector.
@@ -577,6 +580,8 @@
ci--;
free(ci->ci_path, M_DEVBUF);
}
+ if (tmppath != NULL)
+ free(tmppath, M_DEVBUF);
free(cs->sc_cinfo, M_DEVBUF);
return (error);
}
--
Maxim Konovalov, MAcomnet, Internet-Intranet Dept., system engineer
phone: +7 (095) 796-9079, mailto: maxim@macomnet.ru
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-current" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20011230160821.L36208-100000>