[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] [mingw|cygwin] Modify cwrapper to invoke target directly.

From: Charles Wilson
Subject: Re: [PATCH] [mingw|cygwin] Modify cwrapper to invoke target directly.
Date: Sat, 10 May 2008 10:31:17 -0400
User-agent: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv: Gecko/20080421 Thunderbird/ Mnenhy/

Roumen Petrov wrote:
Hi Charles,

About following comment:
/* execv doesn't actually work on mingw as expected on unix */

Actually execXXX on microsoft windows create a new process and it is not mingw problem. What about to substitute "mingw" with "windows" ?

Disagree. The FSF discourages to use the term 'windows' alone to refer to the microsoft operating system (What? GNU-Linux/KDE doesn't have any "windows"?)

"execv doesn't actually work on win32 as expected" implies that cygwin also suffers from this problem, but it doesn't. Ditto on Microsoft Windows as expected...

While as expected when using the native w32api...

Is too wordy, and is probably confusing to many users -- even though it is the most accurate. ("I don't understand this "native w32api" thing -- I'm using mingw!") as expected when using the native w32api (that is, when $host is mingw)...

is marginal, but even more wordy -- and will need changing AGAIN when MSVC support is added (either directly, or via one of the various wrapper script implementations being developed).

Finally, a request: can we please not criticize patches for pre-existing issues in the comments or code, that are not bugs? This comment has been present over a year. With regards to my other, -std=c99 patch, Ralf made similar comments about pre-existing comments, and Eric complained about printf vs. puts -- when the original code also used printf. It's a little difficult to separate the wheat from the chaff, when many of the review comments are not specifically about the patch itself...


reply via email to

[Prev in Thread] Current Thread [Next in Thread]