Re: [Info-mtools] [patch] windows: support Open Watcom V2 compiler (attempt 3)

[avih]

 On Monday, February 3, 2025 at 08:47:09 AM GMT+2, Alain Knaff <alain@knaff.lu> 
wrote:

> On 02/02/2025 14:15, avih wrote:
>
>> - Nit: mcopy still doesn't show: mcopy [-tnvm] MSDOSsourcefile
>>   (I had to check the html docs to understand why it doesn't fail)
>
> Added now

Thanks. I really wasted some time on this. I did it accidentally
once but then wondered why it succeeded, as it's supposed to be
illegal according to the synopsis. I couldn't identify what it did
because the image remained unmodified. Even more, I got a different
result in the next identical invocations ("file already exist").

It just didn't occur to me that the synopsis might be incomplete,
and I only barely noticed that the synopsis at the HTML docs is not
identical to "mcopy -h"...  (then I also noticed the "t" diff etc).


>> [...]
>> direct.h ia an official Windows header which is documented as required
>> for mkdir. I'm guessing that in mingw it gets included indirectly.
>>
>>> Maybe I can better make up my mind if I know:
>>> - if direct.h is still needed for current mtools?
>>> - if yes, for which function prototype(s) is it needed?
>>
>> Yes, it's still required with Watcom for opendir, DIR, etc.
>>
>> opendir is not a Windows API, but it is supported with POSIX semantics
>> in mingw via the posix extension dirent.h, and in Watcom via direct.h
>> (the standard windows header, with opendir and friends added).
>
> I get from this that (in the context of mtools) it would only be needed
> as a fallback if dirent.h is not available.
>
> So, I added a check to configure.ac that checks for direct.h only if
> dirent.h does not exist, and conditionally includes it in unixdir.c
> (same place where dirent.h is included... directory functions are only
> used in unixdir.c)

Well, technically it's always required on Windows for mkdir, but as
noted, it appears to be included indirectly with mingw, so this works.


>> r885 (pointer arithmetic):
>> - there's still this warning in 64 build (long is 32 on Windows):
>> file.c: In function ‘func1’:
>> file.c:632:47: warning: cast from pointer to integer of different size 
>> [-Wpointer-to-int-cast]
>>   632 |         return getAbsCluNr(This) ^ (uint32_t) (unsigned long) 
>>This->head.Next;
>>       |                                               ^
>
> I'm now using uintmax_t for this.

Yeah, good choice. The obvious choice might have been uintptr_t, but
it's optional in C99. size_t also works, but it's not guaranteed to
be big enough, e.g. 64 bit system with 32 bit object size can exist.

For what it's worth, it now has the same warning in 32 bit build,
which is technically correct - the sizes differ, but uintmax_t is
bigger than void* (64/32), so I don't see an issue. Whatever, gcc.

> Intermediary cast was needed to avoid this warning :-)

You should have added another one to the rescue! ;)


Anyway, with the caveats of the .exe extension and the manpages
(which I did get working with texi2roff and your patch, Windows
included), mingw 32 and 64 bit builds now complete cleanly with
current svn trunk r904, and Watcom build succeeds too. Good!

There's one expected warning "Cannot use raw terminal codes".

The Watcom build still has the same warnings as initially (at the
build log which I linked from the original message with the patch),
but I don't see bugs. It can be tightened up a bit if you want:

llong.c(59): Warning! W124: Comparison result always 0
(all good, size_t can be bigger than UINT32_MAX on some systems)

mattrib.c(158): Warning! W107: Missing return value for function 'letterToCode'
(all good, it's NORETURN in this code path)

mbadblocks.c(207): Warning! W136: Comparison equivalent to 'unsigned == 0'
(endSector is always 0 at this line)

mformat.c(934): Warning! W136: Comparison equivalent to 'unsigned == 0'
(redundant: Fs->dir_len is unsigned, and value of 0 remains 0)

mformat.c(939): Warning! W136: Comparison equivalent to 'unsigned == 0'
(Fs->num_fat is unsigned)

partition.c(207): Warning! W136: Comparison equivalent to 'unsigned == 0'
(dev->partition is unsigned)


Overall, the Windows builds are now in much better shape that before:
copying unix-dir works, stdin/stdout can be binary, it can prompt on
name clash when stdin is used for file input, the build is very clean
due to refined types handling and better includes/guards/gnu-ism, and
even supports more compilers and win95 without compiler-specific code!

The other general synopsis and mtype fixes are also great.

(I still want to look into the .exe thing, and try one last time to
convince you to include the modified texi2roff in svn, but not here).

So thanks again for your time and help in making it happen!

Cheers,
Avi