incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Black <abl...@roguewave.com>
Subject Re: [patch] exec utility windows port
Date Tue, 15 Aug 2006 17:30:39 GMT
I believe I posted my patch to the list prior to the referenced change 
being submitted to subversion, but I am uncertain why those particular 
hunks failed to apply (running svn up appears to have merged them 
without conflicts).

Attached is a revised version of the patch that tries to address the 
formatting issues in changed code (a formatting and comment cleanup 
sweep is needed after the windows port is finished).  I have also tried 
to alter the log based on the changes to the patch and the referenced link.

--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.
       (guarded_malloc, guarded_realloc): Allocate memory after asserts.
     * cmdopt.h (escape_code, default_path_sep, suffix_len, suffix_sep): 
Declare (platform specific) file system related constants.
     * cmdopt.cpp (escape_code, default_path_sep, suffix_len, 
suffix_sep): Define above for UNIX systems.
       (split_opt_string): Move use of opts after assert, use 
escape_code as escape character in place of '\'.
     * exec.cpp: (get_signame): Enlarge static buffer, use sprintf in 
place of snprintf.
       (open_input): Use reference_name to determine input file name, 
remove root_len variable, move use of in_root after asserts.
       (exec_file): Alter to use output_name to determine output file name.
     * output.cpp (check_example): Use reference_name to determine 
reference file location, add assert on on_name.
       (parse_output): Use output_name to determine output file name.
     * runall.cpp (merge_argv): Use target after asserts.
       (check_target_ok): Disable (unused) logic for output only 
targets.  Alter compile check on windows systems to correctly locate 
.obj file.
       (rw_basename): Use default_path_sep as separator.
       (run_target): Use target, argv, childargv after asserts.

Martin Sebor wrote:
> 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

Mime
View raw message