Re: casting NULL within a varargs list

[Ben Pfaff]

Thanks.

We're coming pretty close to categorizing and converting all the
casts PSPP uses into macros...

I pushed this as a fix:

--8<--------------------------cut here-------------------------->8--

From: Ben Pfaff <address@hidden>
Date: Thu, 23 Sep 2010 20:18:01 -0700
Subject: [PATCH] cast: New macro NULL_SENTINEL.

---
 src/libpspp/cast.h              |    7 ++++++
 src/output/ascii.c              |   45 ++++++++++++++++++++-------------------
 src/output/options.c            |    2 +-
 src/ui/gui/main.c               |    3 +-
 src/ui/terminal/terminal-opts.c |    3 +-
 5 files changed, 35 insertions(+), 25 deletions(-)

diff --git a/src/libpspp/cast.h b/src/libpspp/cast.h
index 5c64fac..00e42d6 100644
--- a/src/libpspp/cast.h
+++ b/src/libpspp/cast.h
@@ -112,4 +112,11 @@
         (CHECK_POINTER_COMPATIBILITY (&((STRUCT *) 0)->MEMBER, POINTER), \
          (STRUCT *) ((char *) (POINTER) - offsetof (STRUCT, MEMBER)))
 
+/* A null pointer constant suitable for use in a varargs parameter list.
+
+   This is useful because a literal 0 may not have the same width as a null
+   pointer.  NULL by itself is also insufficient because in C it may expand to
+   simply 0. */
+#define NULL_SENTINEL ((void *) NULL)
+
 #endif /* libpspp/cast.h */
diff --git a/src/output/ascii.c b/src/output/ascii.c
index 12cde8e..c9f96b2 100644
--- a/src/output/ascii.c
+++ b/src/output/ascii.c
@@ -22,27 +22,28 @@
 #include <stdint.h>
 #include <stdlib.h>
 
-#include <data/file-name.h>
-#include <data/settings.h>
-#include <libpspp/assertion.h>
-#include <libpspp/compiler.h>
-#include <libpspp/message.h>
-#include <libpspp/start-date.h>
-#include <libpspp/string-map.h>
-#include <libpspp/version.h>
-#include <output/cairo.h>
-#include <output/chart-item-provider.h>
-#include <output/message-item.h>
-#include <output/options.h>
-#include <output/tab.h>
-#include <output/text-item.h>
-#include <output/driver-provider.h>
-#include <output/render.h>
-#include <output/table-item.h>
-
-#include "error.h"
-#include "minmax.h"
-#include "xalloc.h"
+#include "data/file-name.h"
+#include "data/settings.h"
+#include "libpspp/assertion.h"
+#include "libpspp/cast.h"
+#include "libpspp/compiler.h"
+#include "libpspp/message.h"
+#include "libpspp/start-date.h"
+#include "libpspp/string-map.h"
+#include "libpspp/version.h"
+#include "output/cairo.h"
+#include "output/chart-item-provider.h"
+#include "output/driver-provider.h"
+#include "output/message-item.h"
+#include "output/options.h"
+#include "output/render.h"
+#include "output/tab.h"
+#include "output/table-item.h"
+#include "output/text-item.h"
+
+#include "gl/error.h"
+#include "gl/minmax.h"
+#include "gl/xalloc.h"
 
 #include "gettext.h"
 #define _(msgid) gettext (msgid)
@@ -184,7 +185,7 @@ ascii_create (const char *file_name, enum 
settings_output_devices device_type,
                             "bold", EMPH_BOLD,
                             "underline", EMPH_UNDERLINE,
                             "none", EMPH_NONE,
-                            NULL);
+                            NULL_SENTINEL);
 
   a->chart_file_name = parse_chart_file_name (opt (d, o, "charts", file_name));
 
diff --git a/src/output/options.c b/src/output/options.c
index e0259f0..694ffc2 100644
--- a/src/output/options.c
+++ b/src/output/options.c
@@ -145,7 +145,7 @@ parse_boolean (struct driver_option *o)
    O has no user-specified value, then O's default value is treated the same
    way.  If the default value still does not match, parse_enum() returns 0.
 
-   Example: parse_enum (o, "a", 1, "b", 2, NULL) returns 1 if O's
+   Example: parse_enum (o, "a", 1, "b", 2, NULL_SENTINEL) returns 1 if O's
    value if "a", 2 if O's value is "b".
 
    Destroys O. */
diff --git a/src/ui/gui/main.c b/src/ui/gui/main.c
index a78d248..0c88204 100644
--- a/src/ui/gui/main.c
+++ b/src/ui/gui/main.c
@@ -23,6 +23,7 @@
 
 #include "libpspp/argv-parser.h"
 #include "libpspp/assertion.h"
+#include "libpspp/cast.h"
 #include "libpspp/getl.h"
 #include "libpspp/version.h"
 #include "libpspp/copyleft.h"
@@ -145,7 +146,7 @@ startup_option_callback (int id, void *show_splash_)
     case OPT_VERSION:
       version_etc (stdout, "psppire", PACKAGE_NAME, PACKAGE_VERSION,
                    "Ben Pfaff", "John Darrington", "Jason Stover",
-                   NULL);
+                   NULL_SENTINEL);
       exit (EXIT_SUCCESS);
 
     case OPT_NO_SPLASH:
diff --git a/src/ui/terminal/terminal-opts.c b/src/ui/terminal/terminal-opts.c
index 929fd38..d885cd0 100644
--- a/src/ui/terminal/terminal-opts.c
+++ b/src/ui/terminal/terminal-opts.c
@@ -28,6 +28,7 @@
 #include "language/syntax-file.h"
 #include "libpspp/argv-parser.h"
 #include "libpspp/assertion.h"
+#include "libpspp/cast.h"
 #include "libpspp/compiler.h"
 #include "libpspp/getl.h"
 #include "libpspp/llx.h"
@@ -273,7 +274,7 @@ terminal_option_callback (int id, void *to_)
     case OPT_VERSION:
       version_etc (stdout, "pspp", PACKAGE_NAME, PACKAGE_VERSION,
                    "Ben Pfaff", "John Darrington", "Jason Stover",
-                   NULL);
+                   NULL_SENTINEL);
       exit (EXIT_SUCCESS);
 
     default:
-- 
1.7.1



John Darrington <address@hidden> writes:

> Ok.  Thanks for pointing that out.
>
> J'
>
> On Tue, Sep 21, 2010 at 10:11:04PM -0700, Ben Pfaff wrote:
>      The recent commit c75794cff "Const casts" removes some casts of
>      NULL to a pointer type within varargs parameter lists.  This will
>      cause problems on 64-bit systems that #define NULL to 0 (without
>      any cast to pointer type).  This is a special case where the cast
>      cannot be removed.  (If you want to introduce a macro for this
>      case, to make it obvious what's going on, that's fine with me.)
>      
>      The GNU coding standards even has a specific exception for this:
>      
>          Zero without a cast is perfectly fine as a null
>          pointer constant, except when calling a varargs function.
>      
>      (For what it's worth, I prefer NULL over 0 as a null pointer
>      constant.  It makes it obvious that it's a pointer instead of an
>      integer.)
>      
>      -- 
>      Ben Pfaff 
>      http://benpfaff.org

-- 
Ben Pfaff 
http://benpfaff.org