[Emacs-diffs] /srv/bzr/emacs/trunk r108941: Improve static checking when configured --with-ns.

[Paul Eggert]

------------------------------------------------------------
revno: 108941
committer: Paul Eggert <address@hidden>
branch nick: trunk
timestamp: Sat 2012-07-07 12:23:41 -0700
message:
  Improve static checking when configured --with-ns.
  
  See Samuel Bronson's remarks in
  <http://lists.gnu.org/archive/html/emacs-devel/2012-07/msg00146.html>.
  * configure.in (WARN_CFLAGS): Omit -Wunreachable-code, as it's
  a no-op with recent GCC and harmful in earlier ones.
  Omit -Wsync-nand, as it's irrelevant to Emacs and provokes a
  warning when compiling with ObjC.  Always omit
  -Wunsafe-loop-optimizations, as we don't mind when optimization is
  being done correctly.
  
  Fix some minor --with-ns problems found by static checking.
  * src/frame.c (Ftool_bar_pixel_width) [!FRAME_TOOLBAR_WIDTH]:
  (x_set_font) [!HAVE_X_WINDOWS]:
  * src/image.c (xpm_load_image) [HAVE_NS]:
  (x_to_xcolors) [!HAVE_X_WINDOWS && !HAVE_NTGUI]:
  (x_disable_image) [!HAVE_NS && !HAVE_NTGUI]:
  Remove unused local.
  (Fx_parse_geometry) [HAVE_NS]: Don't return garbage.
  (xpm_load_image) [HAVE_NS && !HAVE_XPM]: Remove unused label.
  * src/image.c (x_create_bitmap_from_file) [HAVE_NS]:
  (xpm_load_image, xpm_load) [HAVE_NS && !HAVE_XPM]:
  * src/nsselect.m (symbol_to_nsstring, ns_string_to_pasteboard_internal):
  * src/xfaces.c (Fx_load_color_file) [!HAVE_X_WINDOWS]:
  Fix pointer signedness problem.
  * src/xfaces.c (FRAME_X_FONT_TABLE):
  * src/xterm.h (FRAME_X_FONT_TABLE): Remove unused, incompatible macros.
modified:
  ChangeLog
  configure.in
  src/ChangeLog
  src/frame.c
  src/image.c
  src/nsselect.m
  src/xfaces.c
  src/xterm.h

=== modified file 'ChangeLog'
--- a/ChangeLog 2012-07-07 18:16:15 +0000
+++ b/ChangeLog 2012-07-07 19:23:41 +0000
@@ -1,3 +1,15 @@
+2012-07-07  Paul Eggert  <address@hidden>
+
+       Improve static checking when configured --with-ns.
+       See Samuel Bronson's remarks in
+       <http://lists.gnu.org/archive/html/emacs-devel/2012-07/msg00146.html>.
+       * configure.in (WARN_CFLAGS): Omit -Wunreachable-code, as it's
+       a no-op with recent GCC and harmful in earlier ones.
+       Omit -Wsync-nand, as it's irrelevant to Emacs and provokes a
+       warning when compiling with ObjC.  Always omit
+       -Wunsafe-loop-optimizations, as we don't mind when optimization is
+       being done correctly.
+
 2012-07-07  Glenn Morris  <address@hidden>
 
        * configure.in (BROKEN_SA_RESTART): Doc fix.

=== modified file 'configure.in'
--- a/configure.in      2012-07-07 18:16:15 +0000
+++ b/configure.in      2012-07-07 19:23:41 +0000
@@ -662,16 +662,19 @@
   nw="$nw -Wsign-conversion"        # Too many warnings for now
   nw="$nw -Woverlength-strings"     # Not a problem these days
   nw="$nw -Wtraditional-conversion" # Too many warnings for now
+  nw="$nw -Wunreachable-code"       # so buggy that it's now silently ignored
   nw="$nw -Wpadded"                 # Our structs are not padded
-  nw="$nw -Wredundant-decls"        # We regularly (re)declare getenv etc.
+  nw="$nw -Wredundant-decls"        # we regularly (re)declare functions
   nw="$nw -Wlogical-op"             # any use of fwrite provokes this
-  nw="$nw -Wformat-nonliteral"      # Emacs does this a lot
+  nw="$nw -Wformat-nonliteral"      # we do this a lot
   nw="$nw -Wvla"                    # warnings in gettext.h
   nw="$nw -Wnested-externs"         # use of XARGMATCH/verify_function__
   nw="$nw -Wswitch-enum"            # Too many warnings for now
   nw="$nw -Wswitch-default"         # Too many warnings for now
-  nw="$nw -Wfloat-equal"            # e.g., ftoastr.c
-  nw="$nw -Winline"                 # e.g., dispnew.c's inlining of row_equal_p
+  nw="$nw -Wfloat-equal"            # warns about high-quality code
+  nw="$nw -Winline"                 # OK to ignore 'inline'
+  nw="$nw -Wsync-nand"              # irrelevant here, and provokes ObjC 
warning
+  nw="$nw -Wunsafe-loop-optimizations" # OK to suppress unsafe optimizations
 
   # Emacs doesn't care about shadowing; see
   # <http://lists.gnu.org/archive/html/emacs-diffs/2011-11/msg00265.html>.
@@ -683,12 +686,6 @@
   nw="$nw -Wsuggest-attribute=const"
   nw="$nw -Wsuggest-attribute=pure"
 
-  # Some loops can't be optimized with -O1,
-  # so remove -Wunsafe-loop-optimizations.
-  if echo "$CFLAGS" | $EGREP 'O1' 1>/dev/null; then
-    nw="$nw -Wunsafe-loop-optimizations"
-  fi
-
   gl_MANYWARN_ALL_GCC([ws])
   gl_MANYWARN_COMPLEMENT([ws], [$ws], [$nw])
   for w in $ws; do

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2012-07-07 18:06:32 +0000
+++ b/src/ChangeLog     2012-07-07 19:23:41 +0000
@@ -1,3 +1,22 @@
+2012-07-07  Paul Eggert  <address@hidden>
+
+       Fix some minor --with-ns problems found by static checking.
+       * frame.c (Ftool_bar_pixel_width) [!FRAME_TOOLBAR_WIDTH]:
+       (x_set_font) [!HAVE_X_WINDOWS]:
+       * image.c (xpm_load_image) [HAVE_NS]:
+       (x_to_xcolors) [!HAVE_X_WINDOWS && !HAVE_NTGUI]:
+       (x_disable_image) [!HAVE_NS && !HAVE_NTGUI]:
+       Remove unused local.
+       (Fx_parse_geometry) [HAVE_NS]: Don't return garbage.
+       (xpm_load_image) [HAVE_NS && !HAVE_XPM]: Remove unused label.
+       * image.c (x_create_bitmap_from_file) [HAVE_NS]:
+       (xpm_load_image, xpm_load) [HAVE_NS && !HAVE_XPM]:
+       * nsselect.m (symbol_to_nsstring, ns_string_to_pasteboard_internal):
+       * xfaces.c (Fx_load_color_file) [!HAVE_X_WINDOWS]:
+       Fix pointer signedness problem.
+       * xfaces.c (FRAME_X_FONT_TABLE):
+       * xterm.h (FRAME_X_FONT_TABLE): Remove unused, incompatible macros.
+
 2012-07-07  Glenn Morris  <address@hidden>
 
        * lread.c (load_path_check): New function, split from init_lread.

=== modified file 'src/frame.c'
--- a/src/frame.c       2012-07-05 18:35:48 +0000
+++ b/src/frame.c       2012-07-07 19:23:41 +0000
@@ -2504,16 +2504,13 @@
 used.  */)
   (Lisp_Object frame)
 {
-  struct frame *f;
-
   if (NILP (frame))
     frame = selected_frame;
   CHECK_FRAME (frame);
-  f = XFRAME (frame);
 
 #ifdef FRAME_TOOLBAR_WIDTH
-  if (FRAME_WINDOW_P (f))
-    return make_number (FRAME_TOOLBAR_WIDTH (f));
+  if (FRAME_WINDOW_P (XFRAME (frame)))
+    return make_number (FRAME_TOOLBAR_WIDTH (XFRAME (frame)));
 #endif
   return make_number (0);
 }
@@ -3158,8 +3155,11 @@
 void
 x_set_font (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
 {
-  Lisp_Object font_object, font_param = Qnil;
+  Lisp_Object font_object;
   int fontset = -1;
+#ifdef HAVE_X_WINDOWS
+  Lisp_Object font_param = arg;
+#endif
 
   /* Set the frame parameter back to the old value because we may
      fail to use ARG as the new parameter value.  */
@@ -3170,7 +3170,6 @@
      never fail.  */
   if (STRINGP (arg))
     {
-      font_param = arg;
       fontset = fs_query_fontset (arg, 0);
       if (fontset < 0)
        {
@@ -3201,12 +3200,16 @@
        error ("Unknown fontset: %s", SDATA (XCAR (arg)));
       font_object = XCDR (arg);
       arg = AREF (font_object, FONT_NAME_INDEX);
+#ifdef HAVE_X_WINDOWS
       font_param = Ffont_get (font_object, QCname);
+#endif
     }
   else if (FONT_OBJECT_P (arg))
     {
       font_object = arg;
+#ifdef HAVE_X_WINDOWS
       font_param = Ffont_get (font_object, QCname);
+#endif
       /* This is to store the XLFD font name in the frame parameter for
         backward compatibility.  We should store the font-object
         itself in the future.  */
@@ -3902,7 +3905,7 @@
   (Lisp_Object string)
 {
 #ifdef HAVE_NS
-  call1 (Qns_parse_geometry, string);
+  return call1 (Qns_parse_geometry, string);
 #else
   int geometry, x, y;
   unsigned int width, height;

=== modified file 'src/image.c'
--- a/src/image.c       2012-07-05 18:35:48 +0000
+++ b/src/image.c       2012-07-07 19:23:41 +0000
@@ -323,7 +323,7 @@
   dpyinfo->bitmaps[id - 1].depth = 1;
   dpyinfo->bitmaps[id - 1].height = ns_image_width (bitmap);
   dpyinfo->bitmaps[id - 1].width = ns_image_height (bitmap);
-  strcpy (dpyinfo->bitmaps[id - 1].file, SDATA (file));
+  strcpy (dpyinfo->bitmaps[id - 1].file, SSDATA (file));
   return id;
 #endif
 
@@ -3964,7 +3964,7 @@
            {
              if (xstrcasecmp (SSDATA (XCDR (specified_color)), "None") == 0)
                color_val = Qt;
-             else if (x_defined_color (f, SDATA (XCDR (specified_color)),
+             else if (x_defined_color (f, SSDATA (XCDR (specified_color)),
                                        &cdef, 0))
                color_val = make_number (cdef.pixel);
            }
@@ -4039,7 +4039,6 @@
 
  failure:
   image_error ("Invalid XPM file (%s)", img->spec, Qnil);
- error:
   x_destroy_x_image (ximg);
   x_destroy_x_image (mask_img);
   x_clear_image (f, img);
@@ -4072,7 +4071,7 @@
          return 0;
        }
 
-      contents = slurp_file (SDATA (file), &size);
+      contents = slurp_file (SSDATA (file), &size);
       if (contents == NULL)
        {
          image_error ("Error loading XPM image `%s'", img->spec, Qnil);
@@ -4456,9 +4455,8 @@
   p = colors;
   for (y = 0; y < img->height; ++y)
     {
+#if defined (HAVE_X_WINDOWS) || defined (HAVE_NTGUI)
       XColor *row = p;
-
-#if defined (HAVE_X_WINDOWS) || defined (HAVE_NTGUI)
       for (x = 0; x < img->width; ++x, ++p)
        p->pixel = GET_PIXEL (ximg, x, y);
       if (rgb_p)
@@ -4741,14 +4739,12 @@
   if (n_planes < 2 || cross_disabled_images)
     {
 #ifndef HAVE_NTGUI
-      Display *dpy = FRAME_X_DISPLAY (f);
-      GC gc;
-
 #ifndef HAVE_NS  /* TODO: NS support, however this not needed for toolbars */
 
 #define MaskForeground(f)  WHITE_PIX_DEFAULT (f)
 
-      gc = XCreateGC (dpy, img->pixmap, 0, NULL);
+      Display *dpy = FRAME_X_DISPLAY (f);
+      GC gc = XCreateGC (dpy, img->pixmap, 0, NULL);
       XSetForeground (dpy, gc, BLACK_PIX_DEFAULT (f));
       XDrawLine (dpy, img->pixmap, gc, 0, 0,
                 img->width - 1, img->height - 1);

=== modified file 'src/nsselect.m'
--- a/src/nsselect.m    2012-02-25 06:41:40 +0000
+++ b/src/nsselect.m    2012-07-07 19:23:41 +0000
@@ -62,7 +62,7 @@
   if (EQ (sym, QPRIMARY))     return NXPrimaryPboard;
   if (EQ (sym, QSECONDARY))   return NXSecondaryPboard;
   if (EQ (sym, QTEXT))        return NSStringPboardType;
-  return [NSString stringWithUTF8String: SDATA (XSYMBOL (sym)->xname)];
+  return [NSString stringWithUTF8String: SSDATA (XSYMBOL (sym)->xname)];
 }
 
 static NSPasteboard *
@@ -157,7 +157,7 @@
 
       CHECK_STRING (str);
 
-      utfStr = SDATA (str);
+      utfStr = SSDATA (str);
       nsStr = [[NSString alloc] initWithBytesNoCopy: utfStr
                                              length: SBYTES (str)
                                            encoding: NSUTF8StringEncoding
@@ -388,7 +388,7 @@
       for (rest = Vns_sent_selection_hooks; CONSP (rest); rest = Fcdr (rest))
         call3 (Fcar (rest), selection, target_symbol, successful_p);
     }
-  
+
   return value;
 }
 
@@ -443,7 +443,7 @@
   if (EQ (selection, Qt)) selection = QSECONDARY;
   pb = ns_symbol_to_pb (selection);
   if (pb == nil) return Qnil;
-  
+
   types = [pb types];
   return ([types count] == 0) ? Qnil : Qt;
 }

=== modified file 'src/xfaces.c'
--- a/src/xfaces.c      2012-07-06 21:07:46 +0000
+++ b/src/xfaces.c      2012-07-07 19:23:41 +0000
@@ -233,7 +233,6 @@
 #undef FRAME_X_DISPLAY_INFO
 #define FRAME_X_DISPLAY_INFO FRAME_W32_DISPLAY_INFO
 #define x_display_info w32_display_info
-#define FRAME_X_FONT_TABLE FRAME_W32_FONT_TABLE
 #define check_x check_w32
 #define GCGraphicsExposures 0
 #endif /* WINDOWSNT */
@@ -243,7 +242,6 @@
 #undef FRAME_X_DISPLAY_INFO
 #define FRAME_X_DISPLAY_INFO FRAME_NS_DISPLAY_INFO
 #define x_display_info ns_display_info
-#define FRAME_X_FONT_TABLE FRAME_NS_FONT_TABLE
 #define check_x check_ns
 #define GCGraphicsExposures 0
 #endif /* HAVE_NS */
@@ -6381,7 +6379,7 @@
   CHECK_STRING (filename);
   abspath = Fexpand_file_name (filename, Qnil);
 
-  fp = fopen (SDATA (abspath), "rt");
+  fp = fopen (SSDATA (abspath), "rt");
   if (fp)
     {
       char buf[512];

=== modified file 'src/xterm.h'
--- a/src/xterm.h       2012-07-03 18:24:42 +0000
+++ b/src/xterm.h       2012-07-07 19:23:41 +0000
@@ -736,9 +736,6 @@
 /* This is the Colormap which frame F uses.  */
 #define FRAME_X_COLORMAP(f) FRAME_X_DISPLAY_INFO (f)->cmap
 
-/* This is the 'font_info *' which frame F has.  */
-#define FRAME_X_FONT_TABLE(f) (FRAME_X_DISPLAY_INFO (f)->font_table)
-
 /* The difference in pixels between the top left corner of the
    Emacs window (including possible window manager decorations)
    and FRAME_X_WINDOW (f).  */