[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Modifiable NMERGE in sort
From: |
Jim Meyering |
Subject: |
Re: Modifiable NMERGE in sort |
Date: |
Tue, 01 Apr 2008 08:48:04 +0200 |
"Bo Borgerson" <address@hidden> wrote:
> Okay, I've added some tests and documentation as per your contribution
> guidelines.
Thanks!
One real bug: this should be "sizeof *ord":
> + size_t *ord = xnmalloc(nmerge, sizeof ord);
Some nits:
- s/int nmerge/unsigned int nmerge/
- in comments, refer to "NMERGE" (all caps, per GCS)
From "info standards":
The comment on a function is much clearer if you use the argument
names to speak about the argument values. The variable name itself
should be lower case, but write it in upper case when you are speaking
about the value rather than the variable itself. Thus, "the inode
number NODE_NUM" rather than "an inode".
- always put a space between function name and open-paren.
A few new function uses didn't do that, e.g.,
> + FILE **fps = xnmalloc(nmerge, sizeof *fps);
- no need for this existence check, since the test is run in its
own temporary directory:
> +my $badtmp = 'does/not/exist';
> +-d $badtmp and die "$badtmp exists";
- typo: s/an/and/
+temporary storage utilization at the expense of increased memory usage
+an I/0. Conversely a small value of @var{nmerge} may reduce memory
- Re: Modifiable NMERGE in sort,
Jim Meyering <=