Andrew Black wrote:
Ok...
Lets try splitting some of the issues brought up into a separate patch
(attached, log below). This patch focuses on the snprintf issue, the
use of asserts(), and centralizing filename generation logic. Included
is the file name token abstraction logic. The possibility was raised
that path separators may be more than one character long. I would agree
that it should be considered, but I also feel that to code for this
possibility would slow the program down slightly, for no immediate
benefit, as all current target platforms seem to use a single character
as the directory separator.
Andrew, I'm afraid this patch doesn't apply cleanly -- see below.
I suspect you didn't integrate the following change (output.cpp.rej
is attached): http://svn.apache.org/viewvc?view=rev&revision=431269.
$ patch -d /build/sebor/stdcxx/util/ < ~/tmp/winprep2.diff
Looks like a unified context diff.
The next patch looks like a unified context diff.
The next patch looks like a unified context diff.
The next patch looks like a unified context diff.
Hunk #1 failed at line 232.
Hunk #2 failed at line 242.
2 out of 4 hunks failed: saving rejects to output.cpp.rej
The next patch looks like a unified context diff.
The next patch looks like a unified context diff.
The next patch looks like a unified context diff.
done
Could you please fix that and resubmit the patch?
Also, I would really like you to correct the little formatting
problems that I pointed out in my first feedback. I will not
commit the patch until these have been fixed. I believe a
consistent formatting style is important to the readability
of code and hence to its maintainability and I would rather
not spend time reviewing another round of changes just to
clean up these trivial problems.
--Andrew Black
Log:
* util.h (reference_name, output_name): Declare functions to
generate the names for reference and output files respectively.
* util.cpp (reference_name, output_name): Define above.
* util.cpp (guarded_malloc, guarded_realloc): move use of size asserts
Please do not repeat the file name or the function name part on
subsequent lines. It makes it look like you changed more files
than you actually did.
You might want to give the Change Logs section of the GNU coding
standards a read:
http://www.gnu.org/prep/standards/html_node/Change-Logs.html
Thanks
Martin
***************
*** 232,240 ****
check_example (char* const out_name, FILE* output)
{
struct stat file_info;
- const size_t root_len = strlen (in_root);
- char* const ref_name = (char*)RW_MALLOC (root_len
- + strlen (target_name) + 19);
FILE* reference;
assert (0 != in_root);
--- 232,239 ----
check_example (char* const out_name, FILE* output)
{
struct stat file_info;
+ size_t root_len;
+ char* ref_name;
FILE* reference;
assert (0 != in_root);
***************
*** 242,252 ****
assert (0 != target_name);
assert (0 != output);
/* Try in_root/manual/out/target_name.out */
- memcpy (ref_name, in_root, root_len+1);
- strcat (ref_name, "/manual/out/");
- strcat (ref_name, target_name);
- strcat (ref_name, ".out");
if (0 > stat (ref_name, &file_info)) {
if (ENOENT != errno) {
--- 241,251 ----
assert (0 != target_name);
assert (0 != output);
+ root_len = strlen (in_root);
+
+
/* Try in_root/manual/out/target_name.out */
+ ref_name = reference_name("manual", "out");
if (0 > stat (ref_name, &file_info)) {
if (ENOENT != errno) {
