RE: Bug report: Compile with Microsoft and Intel compiler

[Jerker Bäck]

Reply on reply no 1 on Bug report...

Comment:
I have still not solved the file time macros. There is something wrong
there.


Eli Zaretskii:
> > 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)

> > To support calling conventions:
> > #define C_CALLBACK __cdecl
Agreed on skipping this

> > Disable some trivial annoying warnings:
Forget about it
Could be done with forced header /FI"warning.h" switch
Or /W4 /wd#### /wd#### (intel -Qwd###)

> > make.h(286) error C2373: redefinition;
Yes


> > 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

> > 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

> > dir.c(522):error C4057: differs in indirection to slightly different
base types
> > if (GetVolumeInformation(w32_path,
> The difference between long and unsigned long is certainly not going
> to cause ``serious errors'', especially since these variables are
> never used after the call to GetVolumeInformation.  I don't mind the
> change, but could someone who has Windows development environment
> installed please recheck that some other version of MSVC doesn't use
> long instead of unsigned long?

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*
Valid pointers or NULL: 
[out] lpVolumeSerialNumber - Serial of volume
[out] lpMaximumComponentLength - Max filename length
[out] lpFileSystemFlags - file system flag IDs
      (ie FILE_SUPPORTS_REPARSE_POINTS 0x00000080)
       can be one or more in "OR" combination

OK, the 2 first there's no problem using long (it just look out of style),
the last one I suppose can be OK also, but is not better to use the right
types?

> > 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:

> > 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
 
> > 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

> > 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

> > 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
 
> > 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
What does the cast (struct token * *) do? 
 
> > job.c(540):error C4101: unreferenced local variable

> > job.c(740):error C4701: local variable may be used without having been
initialized
> They are initialized around line 702, so the compiler is wrong.  But
> it's okay to initialize in another place, I guess.
 





 
> > main.c(831):error C4101: unreferenced local variable
> No! it is used on some platforms.  It is better to say
> Btw, doesn't this compiler have fdopen?  If it does, why doesn't it
> use the HAVE_FDOPEN branch?

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?

> > 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.
 
> > 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

> > read.c(2916):error C4210: nonstandard extension used : function given
file scope
> > read.c(2916):error C2373: redefinition; different type modifiers
  
> > read.c(2994):error C4210: nonstandard extension used : function given
file scope
> > extern void dir_setup_glob ();
> No need to change anything, the compiler is wrong.

Well, it fixed it for me

> 
> > remake.c(431):error C4296: expression is always false
> > remake.c(431):error C4307: integral constant overflow
> That's the same problem as before with ORDINARY_MTIME_MAX.  Let's
> revisit that after you find out what is its expansion.


> > remake.c(862):error C4296: expression is always false
> > file->last_mtime = i == 0 ? UNKNOWN_MTIME : NEW_MTIME;
> No solution needed: the compiler is whining about valid code.

 
> > variable.c(285):error C4130: logical operation on address of string
constant
> streq strikes again...

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

???
 
> > glob.c(570):error C4013: func undefined; assuming extern returning int
> Can't we use the prototype for all platforms?  Paul?

> > 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

I agree, but maybe this value will never be negative. Maybe the value is
meant to be processed to a HRESULT with a specific errorcode. 
 
> > sub_proc.c(782):error C4701: local variable may be used without having
been initialized
> > 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

Regards JB