WINDOWS32 missing "m" fix, resubmitted [Original 2 March 2005]

[J. Grant]

The end of this email has the code fix to only skip the first character 
when necessary.

Kind regards
JG

-------- Original Message --------
Subject: Re: make 3.81 MinGW port and testsuite working with MSYS
Date: Wed, 02 Mar 2005 00:19:45 +0000
From: J. Grant <...>
To: Eli Zaretskii <...>
CC: address@hidden
l>

Hi Eli,

on the 27/02/05 04:58, Eli Zaretskii wrote:
> > Date: Sun, 27 Feb 2005 01:06:37 +0000
> > From: "J. Grant" <address@hidden>
> > CC: Eli Zaretskii <address@hidden>,  address@hidden
> >
> > (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