Update of bug #65771 (group make):

                Severity:              3 - Normal => 2 - Minor              
              Item Group:             Enhancement => Build/Install          
             Assigned to:                    None => eliz                   
       Component Version:                    None => SCM                    
           Triage Status:                    None => Small Effort           

    _______________________________________________________

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




    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?65771>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/


Reply via email to