From owner-freebsd-current Sat Apr 20 4:47: 1 2002 Delivered-To: freebsd-current@freebsd.org Received: from mailsrv.otenet.gr (mailsrv.otenet.gr [195.170.0.5]) by hub.freebsd.org (Postfix) with ESMTP id 4D7B537B400 for ; Sat, 20 Apr 2002 04:46:53 -0700 (PDT) Received: from hades.hell.gr (patr530-a217.otenet.gr [212.205.215.217]) by mailsrv.otenet.gr (8.12.2/8.12.2) with ESMTP id g3KBke5q014987; Sat, 20 Apr 2002 14:46:41 +0300 (EEST) Received: from hades.hell.gr (hades [127.0.0.1]) by hades.hell.gr (8.12.2/8.12.2) with ESMTP id g3KBkxNl008859; Sat, 20 Apr 2002 14:46:59 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Received: (from charon@localhost) by hades.hell.gr (8.12.2/8.12.2/Submit) id g3KBkvpx008858; Sat, 20 Apr 2002 14:46:57 +0300 (EEST) (envelope-from keramida@ceid.upatras.gr) Date: Sat, 20 Apr 2002 14:46:56 +0300 From: Giorgos Keramidas To: Chad David Cc: current@FreeBSD.ORG Subject: Re: savecore Message-ID: <20020420114656.GA8171@hades.hell.gr> References: <20020419003134.A54078@colnta.acns.ab.ca> <20020420002817.GD1464@hades.hell.gr> <20020419210045.A87465@colnta.acns.ab.ca> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20020419210045.A87465@colnta.acns.ab.ca> User-Agent: Mutt/1.3.28i 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 On 2002-04-19 21:00, Chad David wrote: > I was actually hoping for a few more comments on the code, but thanks > anyway ;). Nah... Most of the code looks OK, as far as I can tell. I'm not a C guru or something similar, but it is fine. Style things like the two below were what I had written about it in my original mail, but thought they weren't worth the time. What I had written and then removed from the previous message was: + errno = 0; + ret = (int)strtol(buf, NULL, 10); + if (ret == 0) + if (errno == EINVAL || errno == ERANGE) + warnx("invalid value found in bounds, using 0"); I do have to admit seeing two if-statements is a bit weird :) if (ret == 0 && (...)), perhaps? @@ -123,7 +165,20 @@ - goto closefd; + if (force) { ... + } else { + goto closefd; + } } I have to admit I'd write this as: if (force == 0) goto closefd; /* rest of the code with one less indentation level */ But as I said, the code is fine already. But if you do commit the (BUFSIZ * 64) change I mentioned in the earlier post, please do not forget to mention the change and why it's done. If it makes dumps faster to extract, without breaking the existing savecore behavior, it's perfect :) Giorgos Keramidas FreeBSD Documentation Project keramida@{freebsd.org,ceid.upatras.gr} http://www.FreeBSD.org/docproj/ To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-current" in the body of the message