Hi Eli,
on the 27/02/05 04:58, Eli Zaretskii wrote:
Date: Sun, 27 Feb 2005 01:06:37 +0000 From: "J. Grant" <[EMAIL PROTECTED]> CC: Eli Zaretskii <[EMAIL PROTECTED]>, [email protected]
(In my view I do not think we should not include .exe as part of the program output string. The other ports only display "make" or "gmake" etc.)
I'm not sure I understand: do you think we should keep the .exe or do you think we should remove it?
IMHO .exe should not be included in the output text, for the reasons below.
For instance, the output from the MSYS port of make:
$ make -fmissing.mak make: missing.mak: No such file or directory make: *** No rule to make target `missing.mak'. Stop.
In my view the MinGW port should also have the same output, without any OS specific program executable file type suffix.
As I already said, the DJGPP port was showing "make.exe" for years, and it was never a problem. I don't think we should change it now, as it could break something that relies on this, e.g. in sub-make's, or the value of the $MAKE variable. I say, let's leave the .exe alone.
Removing the .exe extension did not cause any increase in test failures (it actually caused 1 failure reduction). I do not think any sub-make's or $MAKE get their values from the global char * program as far as I can see. I had a look through the source, I could not see anything which would be broken by not including ".exe" internally. Do you know of cases in the source code which only function because of the way it is at present with the .exe extension?
In fact on the contary there are several cases where odd filenames such as make.exe.exe internally result when the extra ".exe" is kept in the program name. This is because .exe is added when sub_proc wants to start waiting jobs, when it is picking out the correct program to run. So in my view it is better to not internally contain platform specifics ".exe" in program strings.
See sub_proc.c:318 find_file() to see the function which gets "c:/path/to/make" and then tries in turn to get a valid handle on the .exe, .cmd and .bat extensions of that program, as it does not find, it finally tries without an extension which is the only case that succeedes because "c:/path/to/make.exe.exe", "c:/path/to/make.exe.cmd", "c:/path/to/make.exe.bat", do not find anything.
When I go through the test suite failures I will report back if there are any adverse effects of keeping the ".exe" out of the "program" filename in the common code.
if (program == 0) program = argv[0]; else - ++program; + { + /* Increment past the initial '\', if there is another character after. */ + if(program[0] == '/' || program[0] == '\\') + { + int str_len_p = strlen(program); + if(str_len_p > 1) + ++program; + } + }
Why did you need this snippet? The test for '/' or '\' was already done above, so what case does this handle?
If the environment gave a partial invalid argv[0] containing:
"p:"
Then the additional unchecked ++program;, would mean that program was potentially pointing to an area of memory after the '\0' of "p:". if that happend the program name displayed would be garbage. A quick check on the length of the string before incrementing would avoid a potential corrupt text or crash. (Providing the rest of the string was not stompped on too)
Below is revised code, to check and only try and increment the pointer in the three cases we expect the increment to be required, and then check the lenght is > 1 before incrementing. It also sets a program to "make" if the string not long enough to be valid.
if (program == 0)
program = argv[0];
else
{
/* Increment past the initial '\', if there is another
character after. */
if(program[0] == '/' || program[0] == '\\' || program[0] == ':')
{
int str_len_p = strlen(program);
if(str_len_p > 1)
++program;
else
program = "make";
}
}You might think these checks are going over the top, my view is that it is better to do quick consistency checks like this, in case something from the environment is not being passed in correctly, which would would mean make might crash.
If the argv is broken it might be considered that that is not make's duty to avoid a potential crash though, so if that is the considered view, then this /else/ clause change is not necessary.
Kind regards JG
_______________________________________________ Make-w32 mailing list [email protected] http://lists.gnu.org/mailman/listinfo/make-w32
