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.

8 files changed, 60 insertions, 38 deletions

diff --git a/ChangeLog b/ChangeLog
index f488b7f3aee..414da3e3a8d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2012-07-07  Paul Eggert  <eggert@cs.ucla.edu>
+
+	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  <rgm@gnu.org>
 
 	* configure.in (BROKEN_SA_RESTART): Doc fix.
diff --git a/configure.in b/configure.in
index 59a7130ed49..94314c7d71e 100644
--- a/configure.in
+++ b/configure.in
@@ -662,16 +662,19 @@ else
   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 @@ else
   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
diff --git a/src/ChangeLog b/src/ChangeLog
index 87ea886720c..14d82ee3ecb 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,22 @@
+2012-07-07  Paul Eggert  <eggert@cs.ucla.edu>
+
+	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  <rgm@gnu.org>
 
 	* lread.c (load_path_check): New function, split from init_lread.
diff --git a/src/frame.c b/src/frame.c
index 8db943bd0a5..c0293f6c869 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -2504,16 +2504,13 @@ or right side of FRAME.  If FRAME is omitted, the selected frame is
 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 @@ x_set_screen_gamma (struct frame *f, Lisp_Object new_value, Lisp_Object old_valu
 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 @@ x_set_font (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
      never fail.  */
   if (STRINGP (arg))
     {
-      font_param = arg;
       fontset = fs_query_fontset (arg, 0);
       if (fontset < 0)
 	{
@@ -3201,12 +3200,16 @@ x_set_font (struct frame *f, Lisp_Object arg, Lisp_Object oldval)
 	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 @@ On Nextstep, this just calls `ns-parse-geometry'.  */)
   (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;
diff --git a/src/image.c b/src/image.c
index 4877d262d00..0854d017163 100644
--- a/src/image.c
+++ b/src/image.c
@@ -323,7 +323,7 @@ x_create_bitmap_from_file (struct frame *f, Lisp_Object file)
   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 @@ xpm_load_image (struct frame *f,
 	    {
 	      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 @@ xpm_load_image (struct frame *f,
 
  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 @@ xpm_load (struct frame *f,
 	  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 @@ x_to_xcolors (struct frame *f, struct image *img, int rgb_p)
   p = colors;
   for (y = 0; y < img->height; ++y)
     {
-      XColor *row = p;
-
 #if defined (HAVE_X_WINDOWS) || defined (HAVE_NTGUI)
+      XColor *row = p;
       for (x = 0; x < img->width; ++x, ++p)
 	p->pixel = GET_PIXEL (ximg, x, y);
       if (rgb_p)
@@ -4741,14 +4739,12 @@ x_disable_image (struct frame *f, struct image *img)
   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);
diff --git a/src/nsselect.m b/src/nsselect.m
index 6352d882b7a..a4d91dae1f2 100644
--- a/src/nsselect.m
+++ b/src/nsselect.m
@@ -62,7 +62,7 @@ symbol_to_nsstring (Lisp_Object sym)
   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 @@ ns_string_to_pasteboard_internal (id pb, Lisp_Object str, NSString *gtype)
 
       CHECK_STRING (str);
 
-      utfStr = SDATA (str);
+      utfStr = SSDATA (str);
       nsStr = [[NSString alloc] initWithBytesNoCopy: utfStr
                                              length: SBYTES (str)
                                            encoding: NSUTF8StringEncoding
@@ -388,7 +388,7 @@ On Nextstep, FRAME is unused.  */)
       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 @@ On Nextstep, TERMINAL is unused.  */)
   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;
 }
diff --git a/src/xfaces.c b/src/xfaces.c
index bc42cb4312a..a6b260a2929 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -233,7 +233,6 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #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 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
 #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 @@ where R,G,B are numbers between 0 and 255 and name is an arbitrary string.  */)
   CHECK_STRING (filename);
   abspath = Fexpand_file_name (filename, Qnil);
 
-  fp = fopen (SDATA (abspath), "rt");
+  fp = fopen (SSDATA (abspath), "rt");
   if (fp)
     {
       char buf[512];
diff --git a/src/xterm.h b/src/xterm.h
index 573d8bf966c..86a76fd81a9 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -736,9 +736,6 @@ enum
 /* 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).  */