64-bit compilation and printfs

64-bit compilation and printfs

[Adrian Robert]

[-- Attachment #1: Type: text/plain, Size: 548 bytes --]


In the course of getting the NS port compiling in 64-bit mode, some  
other developers and myself discovered some format - arg mismatches in  
printfs.  XINT and XUINT return EMACS_INT which can be a long under  
LP64.  I'd like to replace places that use %d in the code with %ld,  
and cast the argument to (long) to avoid issues in 32-bit mode.  This  
has been checked in for the NS port; the patch attached here does this  
in common code.  Does anyone think this should be done differently?   
Else I'll check it in as well.

thanks,
Adrian


[-- Attachment #2: sn_64bit_common.patch --]
[-- Type: application/octet-stream, Size: 4770 bytes --]

Index: dbusbind.c
===================================================================
RCS file: /sources/emacs/emacs/src/dbusbind.c,v
retrieving revision 1.47
diff -u -p -r1.47 dbusbind.c
--- dbusbind.c	25 Aug 2009 10:31:22 -0000	1.47
+++ dbusbind.c	12 Oct 2009 00:40:28 -0000
@@ -1247,7 +1247,7 @@ usage: (dbus-method-return-internal BUS 
   CHECK_STRING (service);
   GCPRO3 (bus, serial, service);
 
-  XD_DEBUG_MESSAGE ("%d %s ", XUINT (serial), SDATA (service));
+  XD_DEBUG_MESSAGE ("%lu %s ", (unsigned long)XUINT (serial), SDATA (service));
 
   /* Open a connection to the bus.  */
   connection = xd_initialize (bus);
@@ -1341,7 +1341,7 @@ usage: (dbus-method-error-internal BUS S
   CHECK_STRING (service);
   GCPRO3 (bus, serial, service);
 
-  XD_DEBUG_MESSAGE ("%d %s ", XUINT (serial), SDATA (service));
+  XD_DEBUG_MESSAGE ("%lu %s ", (unsigned long)XUINT (serial), SDATA (service));
 
   /* Open a connection to the bus.  */
   connection = xd_initialize (bus);
Index: font.c
===================================================================
RCS file: /sources/emacs/emacs/src/font.c,v
retrieving revision 1.138
diff -u -p -r1.138 font.c
--- font.c	23 Sep 2009 02:35:54 -0000	1.138
+++ font.c	12 Oct 2009 00:40:33 -0000
@@ -1353,7 +1353,7 @@ font_unparse_xlfd (font, pixel_size, nam
     {
       f[XLFD_AVGWIDTH_INDEX] = alloca (11);
       len += sprintf (f[XLFD_AVGWIDTH_INDEX],
-		      "%d", XINT (AREF (font, FONT_AVGWIDTH_INDEX))) + 1;
+		      "%ld", (long)XINT (AREF (font, FONT_AVGWIDTH_INDEX))) + 1;
     }
   else
     f[XLFD_AVGWIDTH_INDEX] = "*", len += 2;
@@ -1669,7 +1669,7 @@ font_unparse_fcname (font, pixel_size, n
     }
 
   if (INTEGERP (AREF (font, FONT_DPI_INDEX)))
-    len += sprintf (work, ":dpi=%d", XINT (AREF (font, FONT_DPI_INDEX)));
+    len += sprintf (work, ":dpi=%ld", (long)XINT (AREF (font, FONT_DPI_INDEX)));
   if (INTEGERP (AREF (font, FONT_SPACING_INDEX)))
     len += strlen (":spacing=100");
   if (INTEGERP (AREF (font, FONT_AVGWIDTH_INDEX)))
@@ -1682,7 +1682,7 @@ font_unparse_fcname (font, pixel_size, n
       if (STRINGP (val))
 	len += SBYTES (val);
       else if (INTEGERP (val))
-	len += sprintf (work, "%d", XINT (val));
+	len += sprintf (work, "%ld", (long)XINT (val));
       else if (SYMBOLP (val))
 	len += (NILP (val) ? 5 : 4); /* for "false" or "true" */
     }
@@ -1709,9 +1709,10 @@ font_unparse_fcname (font, pixel_size, n
       p += sprintf (p, ":%s=%s", style_names[i],
 		    SDATA (SYMBOL_NAME (styles[i])));
   if (INTEGERP (AREF (font, FONT_DPI_INDEX)))
-    p += sprintf (p, ":dpi=%d", XINT (AREF (font, FONT_DPI_INDEX)));
+    p += sprintf (p, ":dpi=%ld", (long)XINT (AREF (font, FONT_DPI_INDEX)));
   if (INTEGERP (AREF (font, FONT_SPACING_INDEX)))
-    p += sprintf (p, ":spacing=%d", XINT (AREF (font, FONT_SPACING_INDEX)));
+    p += sprintf (p, ":spacing=%ld",
+                  (long)XINT (AREF (font, FONT_SPACING_INDEX)));
   if (INTEGERP (AREF (font, FONT_AVGWIDTH_INDEX)))
     {
       if (XINT (AREF (font, FONT_AVGWIDTH_INDEX)) == 0)
Index: process.c
===================================================================
RCS file: /sources/emacs/emacs/src/process.c,v
retrieving revision 1.596
diff -u -p -r1.596 process.c
--- process.c	30 Aug 2009 05:00:59 -0000	1.596
+++ process.c	12 Oct 2009 00:40:41 -0000
@@ -1522,7 +1522,7 @@ list_processes_1 (query_only)
 	    insert_string ("?");
 	  if (INTEGERP (speed))
 	    {
-	      sprintf (tembuf, " at %d b/s", XINT (speed));
+	      sprintf (tembuf, " at %ld b/s", (long)XINT (speed));
 	      insert_string (tembuf);
 	    }
 	  insert_string (")\n");
Index: xdisp.c
===================================================================
RCS file: /sources/emacs/emacs/src/xdisp.c,v
retrieving revision 1.1313
diff -u -p -r1.1313 xdisp.c
--- xdisp.c	10 Oct 2009 16:39:05 -0000	1.1313
+++ xdisp.c	12 Oct 2009 00:41:06 -0000
@@ -14128,11 +14128,13 @@ try_window_reusing_current_matrix (w)
     return 0;
 
   /* Can't do this if region may have changed.  */
+  /*
   if ((!NILP (Vtransient_mark_mode)
        && !NILP (current_buffer->mark_active))
       || !NILP (w->region_showing)
       || !NILP (Vshow_trailing_whitespace))
     return 0;
+  */
 
   /* If top-line visibility has changed, give up.  */
   if (WINDOW_WANTS_HEADER_LINE_P (w)
@@ -23605,6 +23607,9 @@ note_mouse_highlight (f, x, y)
   if (! EQ (window, dpyinfo->mouse_face_window)
       || (part != ON_TEXT && part != ON_MODE_LINE && part != ON_HEADER_LINE
 	  && !NILP (dpyinfo->mouse_face_window)))
+/*  if ((! EQ (window, dpyinfo->mouse_face_window)
+      || (part != ON_TEXT && part != ON_MODE_LINE && part != ON_HEADER_LINE))
+	  && !NILP (dpyinfo->mouse_face_window))*/
     clear_mouse_face (dpyinfo);
 
   /* Not on a window -> return.  */

[-- Attachment #3: Type: text/plain, Size: 2 bytes --]

Re: 64-bit compilation and printfs

[Eli Zaretskii]

> From: Adrian Robert <adrian.b.robert@gmail.com>
> Date: Sun, 11 Oct 2009 20:46:27 -0400
> 
> In the course of getting the NS port compiling in 64-bit mode, some  
> other developers and myself discovered some format - arg mismatches in  
> printfs.  XINT and XUINT return EMACS_INT which can be a long under  
> LP64.  I'd like to replace places that use %d in the code with %ld,  
> and cast the argument to (long) to avoid issues in 32-bit mode.  This  
> has been checked in for the NS port; the patch attached here does this  
> in common code.  Does anyone think this should be done differently?   
> Else I'll check it in as well.

This is OK, but please note that you seem to have local changes in
xdisp.c unrelated to the %d issue (see below).  Please don't
accidentally check them in as well.

> --- xdisp.c	10 Oct 2009 16:39:05 -0000	1.1313
> +++ xdisp.c	12 Oct 2009 00:41:06 -0000
> @@ -14128,11 +14128,13 @@ try_window_reusing_current_matrix (w)
>      return 0;
>  
>    /* Can't do this if region may have changed.  */
> +  /*
>    if ((!NILP (Vtransient_mark_mode)
>         && !NILP (current_buffer->mark_active))
>        || !NILP (w->region_showing)
>        || !NILP (Vshow_trailing_whitespace))
>      return 0;
> +  */
>  
>    /* If top-line visibility has changed, give up.  */
>    if (WINDOW_WANTS_HEADER_LINE_P (w)
> @@ -23605,6 +23607,9 @@ note_mouse_highlight (f, x, y)
>    if (! EQ (window, dpyinfo->mouse_face_window)
>        || (part != ON_TEXT && part != ON_MODE_LINE && part != ON_HEADER_LINE
>  	  && !NILP (dpyinfo->mouse_face_window)))
> +/*  if ((! EQ (window, dpyinfo->mouse_face_window)
> +      || (part != ON_TEXT && part != ON_MODE_LINE && part != ON_HEADER_LINE))
> +	  && !NILP (dpyinfo->mouse_face_window))*/
>      clear_mouse_face (dpyinfo);

Re: 64-bit compilation and printfs

[Andreas Schwab]

Adrian Robert <adrian.b.robert@gmail.com> writes:

> Index: dbusbind.c
> ===================================================================
> RCS file: /sources/emacs/emacs/src/dbusbind.c,v
> retrieving revision 1.47
> diff -u -p -r1.47 dbusbind.c
> --- dbusbind.c	25 Aug 2009 10:31:22 -0000	1.47
> +++ dbusbind.c	12 Oct 2009 00:40:28 -0000
> @@ -1247,7 +1247,7 @@ usage: (dbus-method-return-internal BUS 
>    CHECK_STRING (service);
>    GCPRO3 (bus, serial, service);
>  
> -  XD_DEBUG_MESSAGE ("%d %s ", XUINT (serial), SDATA (service));
> +  XD_DEBUG_MESSAGE ("%lu %s ", (unsigned long)XUINT (serial), SDATA (service));

The cast should be followed by a space.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: 64-bit compilation and printfs

[Adrian Robert]

On Oct 12, 2009, at 12:03 AM, Eli Zaretskii wrote:

> This is OK, but please note that you seem to have local changes in
> xdisp.c unrelated to the %d issue (see below).  Please don't
> accidentally check them in as well.

Right.. I forgot to exclude those from the diff but don't worry..

Re: 64-bit compilation and printfs

[Stefan Monnier]

> I'd like to replace places that use %d in the code with %ld, and cast
> the argument to (long) to avoid issues in 32-bit mode.

Yes, that's fine, thank you,


        Stefan

Re: 64-bit compilation and printfs

[Dan Nicolaescu]

Adrian Robert <adrian.b.robert@gmail.com> writes:

  > In the course of getting the NS port compiling in 64-bit mode, some
  > other developers and myself discovered some format - arg mismatches in
  > printfs.  XINT and XUINT return EMACS_INT which can be a long under
  > LP64.  I'd like to replace places that use %d in the code with %ld,
  > and cast the argument to (long) to avoid issues in 32-bit mode.  This
  > has been checked in for the NS port; the patch attached here does this
  > in common code.  Does anyone think this should be done differently?

It looks like this the 64-bit mode uses the 32-bit configuration file.
From configure.in:

  ## Apple Darwin / Mac OS X
  *-apple-darwin* )
    case "${canonical}" in
      i[3456]86-* )  machine=intel386 ;;
      powerpc-* )    machine=macppc ;;
      * )            unported=yes ;;
    esac

there should be an:
      x86_64-* )       machine=amdx86-64 ;;
in that "case".

that will allow to remove this from emacs/src/s/intel386.h:

#if defined (DARWIN_OS)
#ifdef _LP64
/* For Intel Mac, with CC='gcc -arch x86_64'.  */
#define NO_ARG_ARRAY
#endif
#endif

there shouldn't be any _LP64 checks in that file.

Re: 64-bit compilation and printfs

[Adrian Robert]

On Oct 14, 2009, at 12:40 PM, Dan Nicolaescu wrote:

> It looks like this the 64-bit mode uses the 32-bit configuration file.
> From configure.in:
>
>  ## Apple Darwin / Mac OS X
>  *-apple-darwin* )
>    case "${canonical}" in
>      i[3456]86-* )  machine=intel386 ;;
>      powerpc-* )    machine=macppc ;;
>      * )            unported=yes ;;
>    esac
>
> there should be an:
>      x86_64-* )       machine=amdx86-64 ;;
> in that "case".
>
> that will allow to remove this from emacs/src/s/intel386.h:
>
> #if defined (DARWIN_OS)
> #ifdef _LP64
> /* For Intel Mac, with CC='gcc -arch x86_64'.  */
> #define NO_ARG_ARRAY
> #endif
> #endif
>
> there shouldn't be any _LP64 checks in that file.

OK, I'm adding the first and taking the second out.  It hasn't fired  
in some time anyway as DARWIN_OS is not defined yet (machine file  
comes before system file in config.h).

However, what is the purpose of the defining of START_FILES in  
amdx86-64.h?  The default case is:

#define START_FILES pre-crt0.o /usr/lib/crt1.o /usr/lib/crti.o
#define LIB_STANDARD -lgcc -lc -lgcc /usr/lib/crtn.o

None of those files are on OS X machines (not sure about non-OS X  
Darwin).

I can exclude it under __APPLE__ but wanted to know why it was there  
first.


-Adrian

Re: 64-bit compilation and printfs

[Dan Nicolaescu]

Adrian Robert <adrian.b.robert@gmail.com> writes:

  > On Oct 14, 2009, at 12:40 PM, Dan Nicolaescu wrote:
  > 
  > > It looks like this the 64-bit mode uses the 32-bit configuration file.
  > > From configure.in:
  > >
  > >  ## Apple Darwin / Mac OS X
  > >  *-apple-darwin* )
  > >    case "${canonical}" in
  > >      i[3456]86-* )  machine=intel386 ;;
  > >      powerpc-* )    machine=macppc ;;
  > >      * )            unported=yes ;;
  > >    esac
  > >
  > > there should be an:
  > >      x86_64-* )       machine=amdx86-64 ;;
  > > in that "case".
  > >
  > > that will allow to remove this from emacs/src/s/intel386.h:
  > >
  > > #if defined (DARWIN_OS)
  > > #ifdef _LP64
  > > /* For Intel Mac, with CC='gcc -arch x86_64'.  */
  > > #define NO_ARG_ARRAY
  > > #endif
  > > #endif
  > >
  > > there shouldn't be any _LP64 checks in that file.
  > 
  > OK, I'm adding the first and taking the second out.  It hasn't fired
  > in some time anyway as DARWIN_OS is not defined yet (machine file
  > comes before system file in config.h).
  > 
  > However, what is the purpose of the defining of START_FILES in
  > amdx86-64.h?  The default case is:
  > 
  > #define START_FILES pre-crt0.o /usr/lib/crt1.o /usr/lib/crti.o
  > #define LIB_STANDARD -lgcc -lc -lgcc /usr/lib/crtn.o
  > 
  > None of those files are on OS X machines (not sure about non-OS X
  > Darwin).
  > 
  > I can exclude it under __APPLE__ but wanted to know why it was there
  > first.

This is a convoluted mess, I'm wondering if anyone has a full
understanding of what is going on with all these flags.

It looks like you are defining START_FILES in darwin.h anyway, so
you can just leave it alone in amdx86-64.h.
Does LIBS_STANDARD get used at all on OS X?  If not, just ignore it
(or #undefine it in darwin.h if it does...)