From stdcxx-dev-return-1949-apmail-incubator-stdcxx-dev-archive=incubator.apache.org@incubator.apache.org Tue Aug 15 01:12:24 2006 Return-Path: Delivered-To: apmail-incubator-stdcxx-dev-archive@www.apache.org Received: (qmail 15421 invoked from network); 15 Aug 2006 01:12:24 -0000 Received: from hermes.apache.org (HELO mail.apache.org) (209.237.227.199) by minotaur.apache.org with SMTP; 15 Aug 2006 01:12:24 -0000 Received: (qmail 57405 invoked by uid 500); 15 Aug 2006 01:12:24 -0000 Delivered-To: apmail-incubator-stdcxx-dev-archive@incubator.apache.org Received: (qmail 57388 invoked by uid 500); 15 Aug 2006 01:12:24 -0000 Mailing-List: contact stdcxx-dev-help@incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: stdcxx-dev@incubator.apache.org Delivered-To: mailing list stdcxx-dev@incubator.apache.org Received: (qmail 57377 invoked by uid 99); 15 Aug 2006 01:12:24 -0000 Received: from asf.osuosl.org (HELO asf.osuosl.org) (140.211.166.49) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 14 Aug 2006 18:12:24 -0700 X-ASF-Spam-Status: No, hits=0.0 required=10.0 tests= X-Spam-Check-By: apache.org Received-SPF: neutral (asf.osuosl.org: local policy) Received: from [208.30.140.160] (HELO moroha.quovadx.com) (208.30.140.160) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 14 Aug 2006 18:12:23 -0700 Received: from qxvcexch01.ad.quovadx.com (qxvcexch01.ad.quovadx.com [192.168.170.59]) by moroha.quovadx.com (8.13.6/8.13.4) with ESMTP id k7F1C0PY018005 for ; Tue, 15 Aug 2006 01:12:00 GMT Received: from [10.70.3.113] ([10.70.3.113]) by qxvcexch01.ad.quovadx.com with Microsoft SMTPSVC(6.0.3790.1830); Mon, 14 Aug 2006 19:12:12 -0600 Message-ID: <44E11F68.7080604@roguewave.com> Date: Mon, 14 Aug 2006 19:12:08 -0600 From: Martin Sebor Organization: Rogue Wave Software User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.7.12) Gecko/20050920 X-Accept-Language: en-us, en MIME-Version: 1.0 To: stdcxx-dev@incubator.apache.org Subject: Re: [patch] exec utility windows port References: <44DA6A90.50006@roguewave.com> <44DA7D4D.7080100@roguewave.com> <44DB9766.4030004@roguewave.com> <44DBDEF7.9030301@roguewave.com> <44DCF336.9030805@roguewave.com> In-Reply-To: <44DCF336.9030805@roguewave.com> Content-Type: multipart/mixed; boundary="------------060004080607040705030502" X-OriginalArrivalTime: 15 Aug 2006 01:12:12.0913 (UTC) FILETIME=[D6E2CE10:01C6C007] X-Virus-Checked: Checked by ClamAV on apache.org X-Spam-Rating: minotaur.apache.org 1.6.2 0/1000/N --------------060004080607040705030502 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit 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 --------------060004080607040705030502 Content-Type: text/plain; name="output.cpp.rej" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="output.cpp.rej" *************** *** 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) { --------------060004080607040705030502--