From owner-freebsd-current Sun Dec 30 5:46:19 2001 Delivered-To: freebsd-current@freebsd.org Received: from relay1.macomnet.ru (relay1.macomnet.ru [195.128.64.10]) by hub.freebsd.org (Postfix) with ESMTP id BD37037B420; Sun, 30 Dec 2001 05:45:44 -0800 (PST) Received: from news1.macomnet.ru (maxim@news1.macomnet.ru [195.128.64.14]) by relay1.macomnet.ru (8.11.3/8.11.3) with ESMTP id fBUDjVY4178313; Sun, 30 Dec 2001 16:45:31 +0300 (MSK) Date: Sun, 30 Dec 2001 16:45:31 +0300 (MSK) From: Maxim Konovalov To: Sheldon Hearn Cc: Poul-Henning Kamp , , Subject: Re: Junior Kernel Hacker Task: ccdinit stack usage. In-Reply-To: <89119.1009717456@axl.seasidesoftware.co.za> Message-ID: <20011230160821.L36208-100000@news1.macomnet.ru> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: owner-freebsd-current@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG 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