[ET Trac] [Einstein Toolkit] #1743: Reduce number of output files per directory
Einstein Toolkit
trac-noreply at einsteintoolkit.org
Fri Feb 13 08:20:32 CST 2015
#1743: Reduce number of output files per directory
-----------------------+----------------------------------------------------
Reporter: eschnett | Owner:
Type: defect | Status: review
Priority: unset | Milestone:
Component: Other | Version: development version
Resolution: | Keywords:
-----------------------+----------------------------------------------------
Comment (by eschnett):
Replying to [comment:7 knarf]:
> Just looking at the diff:
>
> - Why is the case '1' excluded fr om the range for
processes_per_directory?
This is a special case, since {{{ilog(0)}}} is not defined. Put
differently: We need to create at least 0 subdirectories, numbers smaller
than 0
don't make sense.
> - lines 234 and 235 contain commented code. Is there a reason these
should stay?
Leftovers from pre-C99 code -- thanks.
> - The patch uses strcpy/strcat; it should use strncpy/strncat instead.
The assert about the length before
> isn't enough; asserts can be no-ops, and they wouldn't silence
warnings as well.
That's a common misconception. The semantics of {{{strncpy}}} and
{{{strncat}}} are not what people think. The length argument of
{{{strncat}}} isn't the available buffer size, and {{{strncpy}}} does not
always append a {{{NUL}}} character.
> Style: at first I was put off by the use of a space between a function
name and the opening parenthesis, which is allowed, but (at least to me)
unusual for C/C++ code. I later realized that at least in these files this
usage seems to be common. Well - better to stick to one style than to have
none.
>
> (I usually follow: Control statements should have one space between the
control keyword and opening parenthesis, to distinguish them from function
calls.)
--
Ticket URL: <https://trac.einsteintoolkit.org/ticket/1743#comment:10>
Einstein Toolkit <http://einsteintoolkit.org>
The Einstein Toolkit
More information about the Trac
mailing list