bugmake - Bugs: bug #65771, [PATCH] restore Visual C 6 and...

 
 

bug #65771: [PATCH] restore Visual C 6 and newer but older VC 200X builds

Submitter:  Dan D <bulk88>
Submitted:  Thu 23 May 2024 12:09:44 AM UTC
   
 
Severity:  2 - Minor Item Group:  Build/Install
Status:  None Privacy:  Public
Assigned to:  eliz Open/Closed:  Open
Component Version:  SCM Operating System:  MS Windows
Fixed Release:  None Triage Status:  Small Effort
* Mandatory Fields

Add a New Comment Rich Markup
   

Mon 27 May 2024 05:33:51 PM UTC, comment #3: 

Revised "restore Visual C 6 and newer but older VC 200X builds"

-more std C types/casts used, no more app specific "_quad_t" type

also added a 2nd patch fixing build/compile time, for gnumake on old or very old (2000s era) Mingw GCCs for Win32. Putting it in this ticket, because I think it won't apply unless "restore VC6" patch is applied. Commit history looks better if its 2 separate commits in a row.

So patch #2, for old Mingw GCC, this patch also restores RUNTIME execution of gnumake+any Mingw GCC, modern or old!!! on WinXP. Mingw GCC+gnumake binaries, would not execute at all, by WinXP OS (Win 7 & up, the gnumake.exe execs fine). Mingw version made no difference for WinXP refusing to exec the .exe, because of the XML manifest file from the email/thread/commit below, is parsed as malformed/unknown schema/XML Tags for WinXP's kernel. So I added the correct back-compat XML stuff, and WinXP+Gnumake+Mingw runs fine on XP now.

https://lists.gnu.org/archive/html/bug-make/2023-03/msg00101.html

Revision: b2bf660abc4611677670b01fcbb0dc94ce2c35f7
Author: Costas Argyris <costas.argyris@gmail.com>
Date: 6/19/2023 9:51:13 AM
Message: Add a UTF-8 resource when building for Windows


(file #56105, file #56106)

Dan D <bulk88>
Sat 25 May 2024 09:43:03 AM UTC, comment #2: 

comment #1:

> Thanks.  Some comments to the proposed patch:
>
> @@ -576,8 +578,8 @@ char *ttyname (int);

>  /* Define {u,}intmax_t if not defined in <stdint.h> or <inttypes.h>. */
>  #if !HAVE_STDINT_H && !HAVE_INTTYPES_H
> -#define intmax_t long long
> -#define uintmax_t unsigned long long
> +#define intmax_t __int64
> +#define uintmax_t unsigned __int64
>  #endif
>
> IMO, these changes should not be unconditional, but conditioned by _MSC_VER.  It makes little sense to use __int64 in a GCC build.


Agree.

> --- a/src/dir.c
> +++ b/src/dir.c
> @@ -1106,7 +1106,7 @@ print_dir_data_base (void)
>  #if MK_OS_W32
>              printf (_("# %s (key %s, mtime %s): could not be opened.\n"),
>                      dir->name, dir->contents->path_key,
> -                    make_ulltoa ((unsigned long long)dir->contents->mtime, buf));
> +                    make_ulltoa ((unsigned _quad_t)dir->contents->mtime, buf));
>
> Sane here.  And I don't understand why you use the _quad_t type here; at the very least you should use unsigned __int64, no?  Similar comments to other uses of _quad_t.
>
> The definition of _quad_t that you suggest in makeint.h is perhaps okay, but then (a) please don't use lower-case symbold that begins with an underscore, as that is generally reserved, and (b) why have both intmax_t/uintmax_t and _quad_t?


file "dir.c" is cross platform, so I was thinking to minimize OS specific code, especially multi line "#ifdef"s. But there is a better way to rewrite it on me doing a 2nd look.

Other FOSS projects uses term "QUAD"/"quad" or VENDORPREFIX_64_T etc in a PP define or typedef, for a portable, MS Win32-okay 64b int type. "int64_t" I think is too new for gnumake's compat standards, all OSes (correct if wrong). Gnumake vs Win32 vs maybe other OSes in 2020s already had compat and portable-problems with "int64_t" when T look in git histoy, which caused "long long" to get added, etc, etc.  I didnt want to introduce >=C99 fallbacks, for old CCs, and the fallbacks have unprefixed C99/ANSI tokens/identifiers. I just invented "_quad_t", I assumed the community would suggest a better label anyways when I post the patch for public review.

I considered, some mistake, either by gnumake devs, or end user patches, IDK what, accidentally the C99 fallback code is compiled on a C99 or 2020s CC, or future CCs, and a gmake 2024 era code snapshot, the gmake build somehow breaks in 2024 or a 2024 gmake code tarball with a future 2030 CC in 2030. So I picked "_quad_t" label so no conflict even if a mistake happen and fallback is built on a C99/modern CC.  I couldn't think of a better label in the patch 1st revision.

"#if _MSC_VER < VC 2010 \n #define int64_t __int64" works, but egh, Ive seen other FOSS projects typically create a VENDOR_64_T, not "typedef signed __int64 int64_t;"

On a second thinking, there is a better fix. That "long long" cast was added in

https://git.savannah.gnu.org/cgit/make.git/commit/?id=6f7fb050b4af7335de259f570b75bf6c217eb1cd

and it already is in a "#ifdef WIN32". So for any "retro Win32 computing" builds, I declare VC6 support is lowest denominator needed. And VC 6 has 64b ints native. You get Win95/NT 3.51 IDK NT 4 compat with VC6, my personal biz req for private code is W2K. gmake already, so for 100% of Win32 32b, all CCs versions, retro and modern already use __int64. Mingw GCC obv must support __int64 for compat.

current Win32 gmake, has "uintmax_t"->unsigned __int64" hard coded. So that should be a (uintmax_t) cast, not (unsigned long long), and not (VENDOR_64T). A zero/sign extend guard, is always good to put in, even if not need. Future code reasons/future user reasons. Current Win32 gmake doesn't need that cast, but obv some Win32 CC was printing console warning, probably on 32b sign extend vs zero extend to 64b hazard. (uintmax_t) cast solves that.

current gmake is

FILE_TIMESTAMP->uintmax_t->Windows unsigned __int64

So a cast is just for CC silence the warning, and a tiny bit future dev error.

Some accident, some end user changes, gmake C code, for his personal built, at some future point, and gmake FILE_TIMESTAMP type is signed 32b, and uintmax_t type is 64b (user mistake), on a 64b OS, atleast do the cast, for zero extend. I will rework it to (uintmax_t).



> --- a/src/job.c
> +++ b/src/job.c
> @@ -22,6 +22,7 @@ this program.  If not, see <https://www.gnu.org/licenses/>.  */
>  /* Default shell to use.  */
>  #if MK_OS_W32
>  # include <windows.h>
> +# include <winsock2.h> /* for WSABASEERR */
>
> I don't understand the need for this.  The following MS documentation page:
>
>    https://learn.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2
>
> says that these errors are also declared in winerror.h, which windows.h includes.  So why did you need an explicit inclusion on winsock2.h?


"WSA" is a prefix for win sockets API. Old VCs/old MS Win SDKs/old headers don't auto #include "winsock.h". I didnt research if old 2000s era Mingw has same header problem. Maybe it does, maybe not. Its irrel. Use socket api? then you need socket header. If VC/SDK 2022 "winerror.h" auto includes "winsock2.h", that is 2020s new by MS. forgot if VC 6 or VC 2003 or both need "winsock2.h"

If brand new/modern Visual C/Windows SDK, automatically "windows.h"->#include->"winsock.h", it is an accident for gnumake only and something specific to Win32 gmake, or MS was being "helpful" with 2020s VC/SDK for public at large, for the public making 2020s era greenfield new apps. It is 2024, assuming "99.999%" of devs write TCPIP apps. Not crazy. If MS intentionally for VC 2022/SDK 2022, auto includes always "winsock.h", its irrel. Anyways old Win CCs need #include winsock.h. And the #include harmless for 2020s CCs.


> +#if (defined(_MSC_VER) && (_MSC_VER < 1900))
> +//must use shim on all < VC 2015 s
> +// https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-snprintf-snprintf-l-snwprintf-snwprintf-l?view=msvc-170
> +#define snprintf _snprintf_c99
>
> First, please don't use C++-style "//" comments.
> And second, why was this needed? (I've read the page which you quote, and I still don't think I understand the need, please explain.)


Will fix comments.

gmake adds use of POSIX snprintf(), added 2022 in

https://git.savannah.gnu.org/cgit/make.git/commit/?id=97e51c0285109ebb6bf8be3d1e21cb0bd5058e5a

"snprintf" per MS docs

https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-snprintf-snprintf-l-snwprintf-snwprintf-l?view=msvc-170

is Windows 10 runtime OS required, or a claimed crash/corruption/overflow risk. Even tho "VC 2015-2022 CRT" is officially approved to run on Win7. I'd rather not research, what the MS doc says and the runtime observed behavior on Win 7, about the snprintf() retval warning in the MS docs. I think the MS docs are indirectly documenting a long-standing bug, VC 2015->VC 2022 ??? patchlevel.

Or there is some headache, being described there, of some permutation of symbol "snprintf()" and MS VC CC 2015->20XX against Win 7/8 clean ISO install (core OS DLLs) or Win 7/8 no-auto-updates, vs something in gmake.exe's build time "EXE Manifest file", or build time gmake.exe's "signed certificate" or gmake.exe's non-existent UWP/MS App Store "flags", vs Windows 7? 8? 10?, vs Windows App Compat engine, or MS has a bug-compat problem, and can't fix, the "snprintf()" broken behaviour, because 3rd party published code now depends on the bug for correct execution. And then, for VC 2015-2022, for ISO C correct retval, something involving, VC 2022 patchlevel*gmake.exe build time PE DLL XML Manifest*OS weekly auto updates*runtime Major OS ver*your .exe's embedded self-signed unapproved "MS App Store" certificate. Its some level of headache.

In any case, "snprintf" symbol/func is 2020s era Visual C (>=2015). So usage "snprintf" is a gmake old VC broken build. 1990s/2000s/2010s Visual C only has "_snprintf", with unique non portable behaviour. I guess older "_snprintf" could be directly dropped into Win32 gmake for old CCs. But I'm thinking its just smarter to defensive code, around the maybe-bug "VC 202X" on "Win 7/8", and fix build break old CC, at same time, same fix. And defend future, gmake code, against, new gmake usage, of "snprintf()", that could assume retval is ISO C/POSIX behaviour, all OSes, when retval is NOT ISO/POSIX correct for Win32.

I don't see in CURRENT gmake code, any exploitable %s user-string-inputs, to snprintf() that would trigger a buffer overflow/no-NUL-char. But I was thinking about unknown future gmake code changes add NEW snprintf() calls, and depend on much more of snprintf() features than now. Defensive code.


> -# include <stddef.h>    /* for intptr_t */
> +# include <stddef.h>    /* for INT_PTR */


> Is INT_PTR indeed defined in stddef.h?  I very much doubt that.


VC 6 headers yes, VC 6 has it in stddef.h, VC 2022 has INT_PTR in basetsd.h. Comment will prob go away in rev #2 of this patch. with other clean up in the patch.

>
>  osync_parse_mutex (const char *mutex)
>  {
>    char *endp;
> -  unsigned long long i;
> +  UINT_PTR i;

>    errno = 0;
> +#if defined _WIN64
>    i = strtoull (mutex, &endp, 16);
> +#else
> +  i = strtoul (mutex, &endp, 16);
> +#endif
>
> Why this change?  It adds an ugly #ifdef for no good reason.  Why not handle the mutex as unsigned long long (a 64-bit type) in both 32-bit and 64-bit builds?


Win32 Mutex is a Kernel Handle, and a Win32 Kernel handles C type, is always and forever equal to a "void *" (32b or 64b), all CCs, all vers, VC+GCC+all. "strtoul" part has to be 32b vs 64b specific. Very remote truncation hazard x64 OS.

https://net-snmp-coders.narkive.com/395WzMVL/windows-msvc-2008-64-bit-and-platform-sdk

Even if "strtoull()" is used on 32b windows, ull() has old VC build break compat problems, and best to not use strtoull() on 32b Win32 gmake.exe, all CCs old or modern.

I agree my new code, and current code, casts/var decls can be cleaned up in osync_parse_mutex(). Type "HANDLE i_or_h = (HANDLE) foo();" is cleaner. "unsigned int i" vs HANDLE i" decls (32b OS), ONLY, has meaning for C step debugging, so a dev  step debugging osync_parse_mutex() can, visually compare "1234" vs int 1234 in a watch tool, but osync_parse_mutex()/gmake hex encode the HANDLE, so HANDLE/void * better type for that C auto.

Why strtoull() on 64b?

https://learn.microsoft.com/en-us/windows/win32/winprog64/interprocess-communication?redirectedfrom=MSDN

says AMD x64 Kernels and x64 user procs, ALWAYS get 32b truncate safe, kernel HANDLEs. But I see issues.

Lets assume x64 "gmake -j8", with x64 child gmake.exe. Or extremely bizzare (I argue user error, someone will argue no), x64 "gmake.exe -j8", starts 32b "gmake.exe" child procs. And passes "8 byte" kernel handles (mutex obj), in a ASCII string, through %ENV% to those 32b gmake.exe's. This should be fine accord to MS.

MS says in 2024/now, "truncate safe" are kernel handles. No bug, no error. So 2024/currently x64 gmake.exe could use safely "strtoul()" in x64, for the mutex (kernel handle), then either store 32b machine val on x64 from strtoul(), to a C 32b int global in a x64 gmake process (4 bytes of RAM saved total, in exchange for very unusual C code, I disagree intentionally truncating). Then LATER still x64 gmake.exe, will always cast, the mutex kernel handle, in that strange 32b C static var, back to native x64 machine type "HANDLE" aka void *, to pass to Win API/Win kernel fn calls anyways.

Just use strtoull() on 64b.

#2 Defensive code, what if MS in future in "Windows 2030" or "Windows 2035" or Windows ARM/RISC-V/not invented yet CPUs, drops 32b x86 WoW emulation/subsystem, and "x64 gmake.exe" with compiled with VC 2005 or VC 2022, suddenly get full length 64b kernel handles, high 32 bit suddenly have 1s, so using strtoul() (32b) on x64 to de/serialize kernel handles to ASCII strings, while okay in 2024 on Win 10 x64 gmake.exe, seems questionable for future-compat. Best to use ull() on 64b, ul() on 32b. Its natural. "#if\n #else\n ul() vs ull()", protects against 2024 x64 gmake.exe suddenly "breaking" 20 years from now.

If 32b gmake.exe -j4 child process, parses a 64b int in a string, from %ENV%, its 100% error case. Let strtoul() 32b do bounds check for high 32b in 64b, and ret EAX 32b 0/-1 to 32b gmake, vs 32b gmake checking for, EDX==0 && EAX > 0 && EAX < 0xFFFFFFFF, EDX register being MS x86 32b ABI for retval 64b in C, with EDX holds high 32b split. For a 32b proc, a faux-HANDLE, stored in a 64b var, with any bits on in upper 32 bits, is always error. A 32b proc on 64b OS can NEVER pass that 64b faux-HANDLE to WoW 32b WinApi kernel functions no matter what. So bizzare case, 64b gmake.exe -j8 starts child 32b gmake.exe'es, in 2024, 32b childs will exec uneventfully, and Windows 2035 with no WOW 32on64 layer, irrel. User can't start their now "legacy" in 2035 bizzare Makefile build process anyway since 32b .exe s can't start.

#3
It would be extra end-user work/difficulty, on what-if "Windows 2035", with 2024 "x64 gmake.exe", that truncates 64b->32b in x64 proc, to have to use"Windows 2035" App Compat flags/wizard/framework, to force 32b kernel handles, to be delivered to a "legacy" in 2024 x64 gmake.exe. Just defense code to ull() on x64 (cur 2024 gmake.exe). The change is only for 32b gmake.exe builds for old CC compat. Don't use truncating ul(), even if "safe" on x64 builds, to save 5 lines of text, in .c file.


> +/*VC 6 doesn't have strtoi64, VC 7/2003 does */
> +#if defined(_MSC_VER) && (_MSC_VER < 1300)
> +/*  from ReactOS /sdk/lib/crt/string/strtoi64.c
> +    https://git.reactos.org/?p=reactos.git;a=blob;f=COPYING3;hb=HEAD
> +*/
> +/* Based on Wine Staging 1.9.9 - dlls/msvcrt/string.c */
>
> If this code is indeed from Wine, I'm not sure we can accept it without them assigning the copyright to us.  That's Paul's decision, obviously, but AFAIK those are the rules of the GNU project regarding non-trivial contributions of code.
>
>


I have other less perfect ideas for strtoi64() VC6 other than a direct re-implementation shim (above code).

While its nice for 2024 gmake built with VC6 to have the strtoi64() shim. A shim covers all retro VCs, on all retro Win OSes. And keep's gmake.exe's on VC's static link MS Libc design for user friendlyness and a "portable" gmake.exe for copy/pasting/distributing between PCs. (and Win32 gmake with VC, the current gmake win32 build script, right now doesn't support a MS dynamic link external CRT build option, my test shows, its NOT "Replace All" button "/MT" -> "/MD" @ build_w32.bat in an IDE, gmake has build fail+IIRC from last week, no console output hang-ed with /MD).

Back compat design issues if a VC 6+gmake+external/alternative MS provided strtoi64() is used. Its possible, but strange to static link MS VC CRT lib, then for 1 func call, load in addr space, DLL MS CRT (msvcr1XX.dll or msvcrt.dll), and still keep gmake.exe as static link LIBC, not just convert default gmake.exe VC build to more common dynamic link to the MS CRT DLL in C:/windows/system32 like gmake.exe with Mingw GCC has done from day 1. But from day 1, Mingw uses version-less universal msvcrt.dll, which solves the big drama about VC 20XX Redistributable installer/dep drama. Just kooky, intentional keeping, 2 CRTs in 1 proc, by design. Its okay IMO if its no-binary vs unusual but works back compat fix in C app. If a VC 6 build has to runtime search for a MS OS supplied DLL for strtoi64(), some more design problems.

My WinXP 32b msvcrt.dll ("not public" but "public", all GCCs use it) has strtoi64() exported.

My Win2k 32b msvcrt.dll does NOT have a strtoi64() NOT GOOD (2000s Mingws probably keep a copy in libgcc_s_dw2-1.dll or libmingwex.dll etc, or secret static link it in, or mingw (2024? or 2010s?) .exe won't start on win2k, its a user problem, not mingw dev problem.

msvcr70.dll 32b has strtoi64() export, 99% chance a Win2k user by now has VC 2002/VC 2003 redistributable. Statistically only way to be missing msvcr70.dll on Win2K by now is VMs with "clean installs" of Win2k. VC6 only gmake, would be having a dep for runtime >=XP, or install MS VC 2002 runtime, <= 2K/98. Reasonable to me. For a ancient VC, on ancient OS. Not old*ancient, but ancient*ancient.

Call path for current gmake code to strtoi64()

"$(word n,text)"->parse_numeric->strtoi64()
"$(wordlist s,e,text)"->parse_numeric->strtoi64()

So user_input > 32b || user_input > size_t on 32b proc, any OS, with gmake, will fatal error with message, or OOM crash instant.

"future" gmake code, I can guess, someone uses parse_numeric() for a new feature gnumake only feature "read 10 bytes starting at 5.1 GB, of 5.5 GB disk file, into make recipie " in a Makefile var/macro/rule/directive,

or gnumake adds some new date/time Makefile math calc features. Then it can be argued, 32b has legit reason, to parse a 64b from user input, then using the 64b int in 32b proc for something that is NOT always a 4GB malloc() call. I'd rather get strtoi64() to exec on VC 6, vs hypthetic special case unique behavior on gmake VC 6 only, vs gmake >=2002 legacy up.

Alt 1:
macro back parse_numeric() to 32b strtoi() all 32b OSes, linux also, b/c gmake will SEGV anyways with a 5GB makefile or 5GB malloc(), and 32b constant -1 from strtol() errors branches/OOM SEGV, even faster than a truncated 64b value, since truncated 64b could silently keep gmake.exe "going" for a few lines of C, then error branch/fatal exit, or invisible failure by gmake.exe for rest of proc.

Alt 2:
macro back parse_numeric() to 32b strtoi() VC 6 32b gmake only, all other CCs, all other versions, aka VC >= 2003 (2002??), no chagges. the user's bad Makefile syntax would maybe cause flip OOM error msg vs other fatal message, vs SEGV behaviour, of ancient VC6 gmake vs ancient VC 2003->modern, errors/SEGVs/out of memory at all, is a different topic

Alt 3:
VC 6 gmake only, with VC 6 static link CRT build (cur default), very clunky, at ever gmake proc start (egh, perf issue), or on runtime hitting the rarely used gmake "$(word)", at makefile "$(word)" parse hit time, calls LoadLibrary('msvcrt.dll') (XP and up) or LoadLibrary('ntdll.dll') (egh, Win 95/98) and fetch-es strtoi64() fn ptr, with maybe a final fall back to searching msvcr70.dll for strtoi64() on Win2k/ME/98 older, which is probably even less used vs WinXP in 2024. VC 2003 I think, publically supports, building obinaries for Win95/NT 4, so I assume VC 2003 LIBC is installable on NT 4/Win 95 up. And gmake+VC 2003+vanilla static link VC CRT, is uneventful and unchanged.

And realistically, clean installs, from a CD, of NT 4/98/2000, in 2024, and a dev/user, wanting 2024 gmake VC6, to run on a clean install, of NT 4/98/2000, no VC 2002/2003 redistributable MS CRT/LIBC. Nope. Just Nope. Any legit user, with an air gapped, legacy Win 98/2000, box, wanting to rebuild any 1990s/2000s era private code, with 2024 gnumake, and they probably run some legacy air gapped biz apps, either in mid 2000s, or by 2024, some user manually installed on airgapped legacy PC, or some other app auto installed with itself, installed the VC 2002 or 2003 runtime on that air gapped legacy dev machine. Its only VC 2005 that started the decade of SxS DLL Manifest version hell for MS CRT/LIBC. 2002/2003 CRTs redist installers I assume are zero drama universally. I personally always use VC 2003+Win2k to maintain my legacy systems in 2024. Me fixing gmake for VC 6 is more academic reasons or research of VC x86 machine code gen and changes over the years to VC's code gen. Plus VC 6 support, covers 100% of any retro/legacy Win32 hobby needs, of the general public. Very tiny "portable code" benefit, since VC 6 compat shakes out code issues, trial by fire, with VC6's bugs, flaws, and limits, for future, brand new hobby Compilers/hobby OSes.

Alt 3 is less clunky if I add to gmake, build option for VC 6->VC 2022, not default!!, build gmake with VC Ver Specific, DLL based MS CRT (MS default/99.999% Win32 use DLL CRT and don't static link the MS VC CRT).

Then, VC6, do NOT static link LIBC, do DLL CRT build. VC 6 only, This way a VC 6 gmake, on XP and up, at runtime, already has "msvcrt.dll" in address space, and GetProcAddress("strtoi64") is cheap. And no "but 2 LIBCs 1 proc" complaints. Note, its 2024, modern code, running on legacy, obv compat on legacy, and prevent bugs, prevent diff behavior legacy vs modern comes way before perf complaints on legacy. 2 MS libcs, 1 proc, is a perf complaint. I call it tiny. I'd rather have a slow binary vs a tool's release notes saying "ver prior to X.Y, not supported EOM".

Alt 4:
have gmake wrap VC 6's static linked MS LIBC 's "atoi64()", I see a atoi64() in my VC 6's "stdlib.h", having gmake on VC 6, do a shim, with a char count and search loop, after calling atoi64(), for next (char *), NotEq ASCII + and - and 0-9, is basic simple. VC6 also have sscanf(), but both need a shim, with loop to search string buffer, for end of integer. So just make shim of VC 6's native atoi64()+shim with char search loop

Alt 5:
write for gmake with VC 6, a no ReactOS code, strtoi64() implementation, decimal ints only, no UTF8 support, no non-latin numbers (even works any OS ??), covers only gmake needs/parse_numeric() needs, code is too basic to have any IP/license issues

Dan D <bulk88>
Thu 23 May 2024 06:05:32 AM UTC, comment #1: 

Thanks.  Some comments to the proposed patch:

@@ -576,8 +578,8 @@ char *ttyname (int);
 
 /* Define {u,}intmax_t if not defined in <stdint.h> or <inttypes.h>. */
 #if !HAVE_STDINT_H && !HAVE_INTTYPES_H
-#define intmax_t long long
-#define uintmax_t unsigned long long
+#define intmax_t __int64
+#define uintmax_t unsigned __int64
 #endif

IMO, these changes should not be unconditional, but conditioned by _MSC_VER.  It makes little sense to use __int64 in a GCC build.

--- a/src/dir.c
+++ b/src/dir.c
@@ -1106,7 +1106,7 @@ print_dir_data_base (void)
 #if MK_OS_W32
             printf (_("# %s (key %s, mtime %s): could not be opened.\n"),
                     dir->name, dir->contents->path_key,
-                    make_ulltoa ((unsigned long long)dir->contents->mtime, buf));
+                    make_ulltoa ((unsigned _quad_t)dir->contents->mtime, buf));

Sane here.  And I don't understand why you use the _quad_t type here; at the very least you should use unsigned __int64, no?  Similar comments to other uses of _quad_t.

The definition of _quad_t that you suggest in makeint.h is perhaps okay, but then (a) please don't use lower-case symbold that begins with an underscore, as that is generally reserved, and (b) why have both intmax_t/uintmax_t and _quad_t?

--- a/src/job.c
+++ b/src/job.c
@@ -22,6 +22,7 @@ this program.  If not, see <https://www.gnu.org/licenses/>.  */
 /* Default shell to use.  */
 #if MK_OS_W32
 # include <windows.h>
+# include <winsock2.h> /* for WSABASEERR */

I don't understand the need for this.  The following MS documentation page:

   https://learn.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2

says that these errors are also declared in winerror.h, which windows.h includes.  So why did you need an explicit inclusion on winsock2.h?

+#if (defined(_MSC_VER) && (_MSC_VER < 1900))
+//must use shim on all < VC 2015 s
+// https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/snprintf-snprintf-snprintf-l-snwprintf-snwprintf-l?view=msvc-170
+#define snprintf _snprintf_c99

First, please don't use C++-style "//" comments.
And second, why was this needed? (I've read the page which you quote, and I still don't think I understand the need, please explain.)

-# include <stddef.h>    /* for intptr_t */
+# include <stddef.h>    /* for INT_PTR */

Is INT_PTR indeed defined in stddef.h?  I very much doubt that.

 osync_parse_mutex (const char *mutex)
 {
   char *endp;
-  unsigned long long i;
+  UINT_PTR i;
 
   errno = 0;
+#if defined _WIN64
   i = strtoull (mutex, &endp, 16);
+#else
+  i = strtoul (mutex, &endp, 16);
+#endif

Why this change?  It adds an ugly #ifdef for no good reason.  Why not handle the mutex as unsigned long long (a 64-bit type) in both 32-bit and 64-bit builds?

+/*VC 6 doesn't have strtoi64, VC 7/2003 does */
+#if defined(_MSC_VER) && (_MSC_VER < 1300)
+/*  from ReactOS /sdk/lib/crt/string/strtoi64.c
+    https://git.reactos.org/?p=reactos.git;a=blob;f=COPYING3;hb=HEAD
+*/
+/* Based on Wine Staging 1.9.9 - dlls/msvcrt/string.c */

If this code is indeed from Wine, I'm not sure we can accept it without them assigning the copyright to us.  That's Paul's decision, obviously, but AFAIK those are the rules of the GNU project regarding non-trivial contributions of code.


Eli Zaretskii <eliz>
Group Member
Thu 23 May 2024 12:09:44 AM UTC, original submission:  

see patch

Dan D <bulk88>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by eliz (Posted a comment)
  • -email is unavailable- added by bulk88 (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 8 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2024-05-27 bulk88 Attached File- Added 0001-restore-Visual-C-6-and-newer-but-older-VC-200X-build.patch, #56105
        Attached File- Added 0002-Win32-fix-build-for-compat-with-mid-2000s-Mingw-GCCs.patch, #56106
    2024-05-23 eliz Severity3 - Normal 2 - Minor
        Item GroupEnhancement Build/Install
        Assigned toNone eliz
        Component VersionNone SCM
        Triage StatusNone Small Effort
    2024-05-23 bulk88 Attached File- Added 0001-restore-Visual-C-6-and-newer-but-older-VC-200X-build.patch, #56078

    Back to the top

    Powered by Savane 3.14-fc07.
    Corresponding source code