Date: Sat, 20 Apr 2002 14:46:56 +0300 From: Giorgos Keramidas <keramida@ceid.upatras.gr> To: Chad David <davidc@acns.ab.ca> Cc: current@FreeBSD.ORG Subject: Re: savecore Message-ID: <20020420114656.GA8171@hades.hell.gr> In-Reply-To: <20020419210045.A87465@colnta.acns.ab.ca> References: <20020419003134.A54078@colnta.acns.ab.ca> <20020420002817.GD1464@hades.hell.gr> <20020419210045.A87465@colnta.acns.ab.ca>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020420114656.GA8171>