FACE_TTY_DEFAULT_* redefined in dispextern.h

FACE_TTY_DEFAULT_* redefined in dispextern.h

[Francesco Potorti`]

While compiling on  x86_64-unknown-linux-gnu with
Debian testing with gcc (GCC) 4.1.2 20061028 (prerelease) (Debian 4.1.1-19)

I get:

gcc -c -D_BSD_SOURCE   -Demacs -DHAVE_CONFIG_H   -I. -I/home/pot/gnu/emacs-22.0.91/src -D_BSD_SOURCE  -g -O2 -Wno-pointer-sign  dispnew.c
dispnew.c: In function 'init_display':
dispnew.c:6895: warning: overflow in implicit constant conversion
dispnew.c:6896: warning: overflow in implicit constant conversion

This is due to these lines in dispnew.c:
      FRAME_FOREGROUND_PIXEL (sf) = FACE_TTY_DEFAULT_FG_COLOR;
      FRAME_BACKGROUND_PIXEL (sf) = FACE_TTY_DEFAULT_BG_COLOR;

which expand to:
      ((sf)->output_data.x->foreground_pixel) = ((unsigned long) -2);
      ((sf)->output_data.x->background_pixel) = ((unsigned long) -3);

foreground_pixel and background_pixel are int (size 4) on my system, 
while unsigned long is size 8.

I applied the appended patch to my local version, which makes the warning
disappear:

================================================================
--- /home/pot/gnu/emacs-22.0.91/src/dispextern.h~	2006-10-11 16:14:02.000000000 +0200
+++ /home/pot/gnu/emacs-22.0.91/src/dispextern.h	2006-12-02 08:43:24.000000000 +0100
@@ -1570,15 +1570,15 @@ struct face
 
 /* Color index indicating that face uses a terminal's default color.  */
 
-#define FACE_TTY_DEFAULT_COLOR ((unsigned long) -1)
+#define FACE_TTY_DEFAULT_COLOR ((PIX_TYPE) ((unsigned long) -1))
 
 /* Color index indicating that face uses an unknown foreground color.  */
 
-#define FACE_TTY_DEFAULT_FG_COLOR ((unsigned long) -2)
+#define FACE_TTY_DEFAULT_FG_COLOR ((PIX_TYPE) ((unsigned long) -2))
 
 /* Color index indicating that face uses an unknown background color.  */
 
-#define FACE_TTY_DEFAULT_BG_COLOR ((unsigned long) -3)
+#define FACE_TTY_DEFAULT_BG_COLOR ((PIX_TYPE) ((unsigned long) -3))
 
 /* Non-zero if FACE was realized for unibyte use.  */
================================================================

I have not the time to check that this is the right thing to do.  One
should
- check that FACE_TTY_DEFAULT_* are in fact compared or assigned to
  PIX_TYPE variables
- if there is no reason to convert to unsigned long, simplify the macro
  definitions in dispextern.h with a simple cast to PIX_TYPE

Please answer to me in Cc, as I don't usually read the list.

I should have sent this to emacs-pretest-bug@gnu.org, but I get a bounce
from that list.  Could someone correct this problem?

Re: FACE_TTY_DEFAULT_* redefined in dispextern.h

[Eli Zaretskii]

> This is due to these lines in dispnew.c:
>       FRAME_FOREGROUND_PIXEL (sf) = FACE_TTY_DEFAULT_FG_COLOR;
>       FRAME_BACKGROUND_PIXEL (sf) = FACE_TTY_DEFAULT_BG_COLOR;
> 
> which expand to:
>       ((sf)->output_data.x->foreground_pixel) = ((unsigned long) -2);
>       ((sf)->output_data.x->background_pixel) = ((unsigned long) -3);
> 
> foreground_pixel and background_pixel are int (size 4) on my system, 
> while unsigned long is size 8.

I think the change you are suggesting is not the right one, since
xterm.h has this in its definition of x_output:

    struct x_output
    {
      ...
      unsigned long background_pixel;
      unsigned long foreground_pixel;

I think we should instead redefine PIX_TYPE in this fragment of
frame.h to be unsigned long instead of int:

    #if !defined(MSDOS) && !defined(WINDOWSNT) && !defined(MAC_OS)

    #if !defined(HAVE_X_WINDOWS)

    #define PIX_TYPE int

Could you please see if such a change fixes your problems?

Re: FACE_TTY_DEFAULT_* redefined in dispextern.h

[Francesco Potorti`]

>> This is due to these lines in dispnew.c:
>>       FRAME_FOREGROUND_PIXEL (sf) = FACE_TTY_DEFAULT_FG_COLOR;
>>       FRAME_BACKGROUND_PIXEL (sf) = FACE_TTY_DEFAULT_BG_COLOR;
>> 
>> which expand to:
>>       ((sf)->output_data.x->foreground_pixel) = ((unsigned long) -2);
>>       ((sf)->output_data.x->background_pixel) = ((unsigned long) -3);
>> 
>> foreground_pixel and background_pixel are int (size 4) on my system, 
>> while unsigned long is size 8.
>
>I think the change you are suggesting is not the right one, since
>xterm.h has this in its definition of x_output:
>
>    struct x_output
>    {
>      ...
>      unsigned long background_pixel;
>      unsigned long foreground_pixel;
>
>I think we should instead redefine PIX_TYPE in this fragment of
>frame.h to be unsigned long instead of int:
>
>    #if !defined(MSDOS) && !defined(WINDOWSNT) && !defined(MAC_OS)
>
>    #if !defined(HAVE_X_WINDOWS)
>
>    #define PIX_TYPE int
>
>Could you please see if such a change fixes your problems?

I defined PIX_TYPE to be unsigned long rather than int: now no warnings
are elicited while compiling dispnew.c and the compilation process
completes without errors.

Re: FACE_TTY_DEFAULT_* redefined in dispextern.h

[Eli Zaretskii]

> Date: Mon, 04 Dec 2006 11:52:35 +0100
> From: Francesco Potorti` <pot@gnu.org>
> Cc: emacs-devel@gnu.org
> >
> >I think we should instead redefine PIX_TYPE in this fragment of
> >frame.h to be unsigned long instead of int:
> >
> >    #if !defined(MSDOS) && !defined(WINDOWSNT) && !defined(MAC_OS)
> >
> >    #if !defined(HAVE_X_WINDOWS)
> >
> >    #define PIX_TYPE int
> >
> >Could you please see if such a change fixes your problems?
> 
> I defined PIX_TYPE to be unsigned long rather than int: now no warnings
> are elicited while compiling dispnew.c and the compilation process
> completes without errors.

Thanks, I installed that change now.