Re: Bug report: Compile with Microsoft and Intel compiler

[Eli Zaretskii]

> From: =?iso-8859-1?Q?Jerker_B=E4ck?= <address@hidden>
> Cc: <address@hidden>
> Date: Thu, 28 Apr 2005 03:25:14 +0200
> 
> I have still not solved the file time macros. There is something wrong
> there.

I need to see the expansion of those macros to be able to help you
figure it out.  Can you produce such an expansion (it should be a
simple matter of invoking the compiler to produce a preprocessed
source)?

> > > Missing header:
> > > #include <direct.h>    (for chdir)
> > > place in make.h (ie line 364)
> > 
> > Is <direct.h> the only header where chdir's prototype is declared?
> Yes
>  
> > If using <direct.h> is the only way, then it will have to be
> > conditioned on some MS-specific symbol, since this is a non-standard
> > header which other compilers don't have.
> Borland also have direct.h (chdir in dir.h)

This will be solved by crafting a w32/include/unistd.h.

As I said earlier, I need to see the full prototypes of all these
functions as they appear in the Microsoft headers.  Then I will write
the Windows version of unistd.h and ask you to try that.

> > > make.h(481):error C2373: redefinition; different type modifiers
> > > extern char *getcwd ();
> > > make.h(490) error C4273: inconsistent dll linkage
> > > extern char **environ;
> > > commands.c(36):error C2373:redefinition; different type modifiers
> > > extern int getpid ();
> > > fnmatch.c(123): error C2373: redefinition;
> > > extern char *getenv ();
> > > job.c(180):error C2373: redefinition; different type modifiers
> > > extern int dup2 ();
> > > extern int execve ();
> > > extern void _exit ();
> > > main.c(77):error C2373: redefinition; different type modifiers
> > > extern int chdir ();
> > > main.c(823):error C2373: redefinition; different type modifiers
> > > extern char *mktemp PARAMS ((char *template));
> 
> We don't want any these - ever

Right.  Some of them will be solved by unistd.h, for others we will
need to include other headers.

> > > ar.c(301):error C4113:differs in parameter lists
> > > ar.c(301):error C2440:cannot convert from __stdcall to __cdecl
> > Is this problem going to disappear if you invoke the compiler in a way
> > that makes __cdecl the default?  If so, let's compile that way.
> No, warning 4113 is due to 
> #define PARAMS(protos)  ()
> when it should be
> #define PARAMS(protos)  protos
> Look at my solution

Are we sure there's no way of forcing MSC to set __STDC__ to a
non-zero value without disabling the extensions?  Perhaps there's a
compiler switch with that effect?

If not, then I guess we will have to use your solution, although I'd
like to avoid that.

> Win32 API function:
> BOOL GetVolumeInformation(
>   LPCTSTR lpRootPathName,
>   LPTSTR lpVolumeNameBuffer,
>   DWORD nVolumeNameSize,
>   LPDWORD lpVolumeSerialNumber,
>   LPDWORD lpMaximumComponentLength,
>   LPDWORD lpFileSystemFlags,
>   LPTSTR lpFileSystemNameBuffer,
>   DWORD nFileSystemNameSize
> );
> 
> LPDWORD <=> DWORD* <=> unsigned long*

Right, so unsigned long is okay here.

> > > dir.c(1167):error C2373: different type modifiers
> > > extern int stat PARAMS ((const char *path, struct stat *sbuf));
>  
> > > dir.c(639):error C4701: local variable may be used without having been
> initialized
> > No, no, no, the compiler is right here: this is actually a bug, we
> > cannot use st.st_mtime without calling stat first.  Please try the
> > following patch:

Did you try that patch, and did it work?

> > > dir.c(1185):error C4113: differs in parameter lists
> > > dir.c(1186):error C4113: differs in parameter lists
> > > dir.c(1188):error C2440: cannot convert from __stdcall to __cdecl
> > 
> > This will go away if we force the compiler to use __cdecl by default,
> > right?  If so, let's do that.
> No, again the compiler complains about
> 
> #define __P(protos)    ()
> #define __PMT(protos)  ()
> When it should be
> #define __P(protos)   protos
> #define __PMT(protos) protos
> 
> glob.h line 27
> __PMT will not be correctly defined since __P is already defined in hash.h
> Look at solution

Ah, okay, this is a bug in glob.h: they shouldn't assume that if __P
is defined, so is __PMT.  I will submit a patch that will separate
them.

> > > file.c(553):error C4296: expression is always false
> > The first error (about expression being always false) is harmless.
> > It's the second one that bothers me.  What is the definition of
> > uintmax_t (on the inttypes.h header) for your compiler?
> 
> uintmax_t defined in config.h
> #define uintmax_t unsigned long

As I suspected.  I'll know more when you show the expansion of the
ORDINARY_MTIME_MAX macro.

> > > function.c(512):error C4130: logical operation on address of string
> constant
> > Is iAPX286 defined in your build?  If so, I think the funky
> > definitions of streq and strieq conditioned on iAPX286, which try to
> > ``optimize'' strcmp should be tossed and we should use strcmp and
> > strcmpi, as the non-iAPX286 branch does.
> 
> iAPX286 not defined

Right, so your compilation picks up the ``optimized'' definitions of
streq and strieq.  I will submit a patch to remove those funky
definitions.

> > > fnmatch.c(484):error C4028: formal parameters different from declaration
> > That's because the compiler doesn't define __STDC__ to a non-zero
> > value.  Can you find a compiler switch that would force __STDC__ to be
> > non-zero?
> No, not good
> Need to have __const defined

Okay.

> > > hash.c(49):error C4115: named type definition in parentheses
> > > likewise at line 265
> > I don't understand this one: what's wrong with a typecast?  The code
> > it is complaining about looks like this (after macro-expansion):
> > 
> >   ht->ht_vec = (void **) ((struct token * *)
> >                            calloc (sizeof(struct ht->token 
> > *), (ht->ht_size);
> 
> I think the compiler is reffering to struct token
> I can not find any definition of this struct anywere

You are right.  Paul, this is a bug, albeit a harmless one, as long as
we don't dereference any struct token object.

> What does the cast (struct token * *) do? 

In the x86 architecture, nothing at all.

> Yes it have fdopen. Good question.
> Microsoft
> FILE *_fdopen(int fd, const char *mode);  <stdio.h>
> Borland
> FILE *_fdopen(int handle, char *type);    <stdio.h>
> 
> Can not find HAVE_FDOPEN in config.h.w32
> Maybe add this?

We could do that, but you show _fdopen, not fdopen.  Are there
prototypes for fdopen (without the leading underscore) as well, and if
so, on what header?

> 
> > > main.c(1481):error C4210: nonstandard extension used : function given
> file scope
> > I don't see any need to do that, the code is perfectly valid as it
> > is.
> 
> > > main.c(1725):error C4296: expression is always false
> > Something is wrong here, the error message doesn't make sense since
> > the line is not a comparison that yields true or false.  Does the
> > problem go away if you break that line into two, like below?
> > 
> >    f->mtime_before_update = NEW_MTIME;
> >    f->last_mtime = f->mtime_before_update;
> 
> Full error message:
> main.c(1728) : error C4296: '<' : expression is always false
> 
> You are right: The compiler finds something I do not understand.

Ah, now I do understand it: it is complaining about the comparison in
INTEGER_TYPE_SIGNED (which is used to define NEW_MTIME):

  #define INTEGER_TYPE_SIGNED(t) ((t) -1 < 0)

so it's harmless.

> > > read.c(1462):error C4244: conversion, possible loss of data
> > > = (v != 0 && *v->value != '\0') == notdef;
> > > 
> > > solution:
> > > = (char)((v != 0 && *v->value != '\0') == notdef);
> > 
> > I think the compiler complains about the "v != 0" part, in which case
> > we could replace it with "v != NULL".
> The expression evaluates to int => need explicit cast to char

Okay.

> > > glob.c(500):error C4389: signed/unsigned mismatch
> > Sigh...
> 
> ???

The "sigh" was because I _hate_ compilers that whine about
signed/unsigned mismatches.

> > > sub_proc.c(765) : error C4057: differs in indirection to slightly
> different base types
> > > change to
> > > unsigned long exit_code;
> > Are you sure it's "unsigned long", not "unsigned int"?  I'd be surprised.
> 
> Win32 API func:
> BOOL GetExitCodeProcess(
>   HANDLE hProcess,
>   LPDWORD lpExitCode
> );
> The handle must have the PROCESS_QUERY_INFORMATION access right
> lpExitCode must be a valid pointer

Okay, but then please change all the places where exit_code is used to
unsigned long.

> > > HANDLE tStdin = 0, tStdout = 0, tStderr = 0;
> > 
> > Better initialize them to -1, since 0 is a valid handle.
> 
> No No No
> A HANDLE is either a pointer to a valid object OR NULL
> One should allways initialize a HANDLE to NULL
> typedef void* HANDLE

Okay.