Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 19 Apr 2002 21:00:45 -0600
From:      Chad David <davidc@acns.ab.ca>
To:        Giorgos Keramidas <keramida@ceid.upatras.gr>
Cc:        current@FreeBSD.org
Subject:   Re: savecore
Message-ID:  <20020419210045.A87465@colnta.acns.ab.ca>
In-Reply-To: <20020420002817.GD1464@hades.hell.gr>; from keramida@ceid.upatras.gr on Sat, Apr 20, 2002 at 03:28:18AM %2B0300
References:  <20020419003134.A54078@colnta.acns.ab.ca> <20020420002817.GD1464@hades.hell.gr>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Apr 20, 2002 at 03:28:18AM +0300, Giorgos Keramidas wrote:
> On 2002-04-19 00:31, Chad David wrote:
> > Any comments / objections to these patches to savecore and friends?
> 
> Since you asked ... :)

Yes, I did.

> 
> > Index: savecore.8
> > ===================================================================
> 
> > +The
> > +.Nm savecore
> 
> You can safely remove "savecore" from the .Nm arguments,
> since the macro will add it when given no arguments.
> (This occurs in several places below, too.)

I am aware of that, but I prefer not to.  I find it easier to read the
raw page with the name in.  Unless ru says otherwise I'm going to keep
doing it that way :).

> 
> > +Print additional debugging information, including the details of the dump
> > +header to stdout.
> 
> The following sounds a tiny bit better:
> 
> 	+Print additional debugging information to standard output,
> 	+including the details of the dump header.

Works for me.

> 
> > +The
> > +.Nm savecore
> > +command attempts to verify that a core image is valid by verifying it's
> > +header (magic number and version etc.).
> 
> "its header".  No apostrophe.
> You might also like to drop "and" in
> "(magic number, version, etc.)."

Ok.

> 
> >  The ``#'' is the number from the first line of the file
> >  .Ar directory Ns Pa /bounds ,
> > -and it is incremented and stored back into the file each time
> > -.Nm
> > +and is incremented and stored back into the file each time
> > +.Nm savecore
> >  successfully runs.
> 
> Breaking the sentences, makes the whole thing easier to understand,
> and removes the need for the "and" joining those two parts:
> 
> 	The ``#'' is the number from the first line of the file
> 	.Ar directory Ns Pa /bounds .
> 	It is incremented and stored back into the file each time
> 	.Nm
> 	runs successfully.

That was taken directly from savecore.8 on -stable, but I do agree
with you.

> 
> >  If
> > +.Nm savecore
> > +successfully saves the core dump, and the
> > +.Fl k
> > +option is not specific, the dump's header is cleared so that future
> 
> 	s/specific/specified/

Yup.

> 
> >  static void
> >  DoFile(const char *device)
> >  {
> >  	struct kerneldumpheader kdhf, kdhl;
> > -	char buf[BUFSIZ];
> > +	char buf[BUFSIZ * 64];
> 
> 	Is this multiplication really necessary?
> 	Was the original buf[BUFSIZ] size not adequate?

Because on my test machine it saves me time waiting for a 512M dump
to a somewhat slow disk.  I didn't try to tune the value, I just
increased it (I didn't think it was worth the effort).  If somebody
has a better number I'll use it :).


I was actually hoping for a few more comments on the code, but thanks
anyway ;).

-- 
Chad David        davidc@acns.ab.ca
www.FreeBSD.org   davidc@freebsd.org
ACNS Inc.         Calgary, Alberta Canada
Fourthly, The constant breeders, beside the gain of eight shillings
sterling per annum by the sale of their children, will be rid of the
charge of maintaining them after the first year. - Johnathan Swift

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?20020419210045.A87465>