generator: Check parameters are not NULL (RHBZ#501893).

This adds additional tests to check that several types of parameter
including String are not NULL when passed to the C functions.
Previously this would cause a segfault inside libguestfs.  With
this change, you get an error message / exception.

Of the possible pointer parameters, only OptString is now permitted
to be NULL.

This change does not affect the Perl bindings.  This is because Perl
XS code was already adding similar checks if you passed undef into
a parameter expecting a string.

diff --git a/.gitignore b/.gitignore
index 9f487c6..9997356 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -198,6 +198,7 @@ python/bindtests.py
 python/guestfs.py
 python/guestfs-py.c
 python/guestfs.pyc
+regressions/rhbz501893
 regressions/test1.img
 regressions/test.err
 regressions/test.out

diff --git a/po/POTFILES.in b/po/POTFILES.in
index 8086ab2..b233c29 100644 (file)
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -92,6 +92,7 @@ perl/bindtests.pl
 perl/lib/Sys/Guestfs.pm
 perl/lib/Sys/Guestfs/Lib.pm
 python/guestfs-py.c
+regressions/rhbz501893.c
 regressions/test-lvm-mapping.pl
 regressions/test-noexec-stack.pl
 ruby/ext/guestfs/_guestfs.c

diff --git a/regressions/Makefile.am b/regressions/Makefile.am
index 7ec9dc8..385de45 100644 (file)
--- a/regressions/Makefile.am
+++ b/regressions/Makefile.am
@@ -24,6 +24,7 @@
 include $(top_srcdir)/subdir-rules.mk
 
 TESTS = \
+       rhbz501893 \
        rhbz503169c10.sh \
        rhbz503169c13.sh \
        rhbz557655.sh \
@@ -58,6 +59,16 @@ TESTS_ENVIRONMENT = \
        PERL5LIB=$(top_builddir)/perl/blib/lib:$(top_builddir)/perl/blib/arch \
        NOEXEC_CHECK="$(top_builddir)/src/.libs/libguestfs.so $(top_builddir)/daemon/guestfsd"
 
+check_PROGRAMS = \
+       rhbz501893
+
+rhbz501893_SOURCES = rhbz501893.c
+rhbz501893_CFLAGS = \
+       -I$(top_srcdir)/src -I$(top_builddir)/src \
+       $(WARN_CFLAGS) $(WERROR_CFLAGS)
+rhbz501893_LDADD = \
+       $(top_builddir)/src/libguestfs.la
+
 EXTRA_DIST = \
        $(FAILING_TESTS) \
        $(SKIPPED_TESTS) \

diff --git a/regressions/rhbz501893.c b/regressions/rhbz501893.c
new file mode 100644 (file)
index 0000000..54c052d
--- /dev/null
+++ b/regressions/rhbz501893.c
@@ -0,0 +1,50 @@
+/* Regression test for RHBZ#501893.
+ * Test that String parameters are checked for != NULL.
+ * Copyright (C) 2009-2010 Red Hat Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+ */
+
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <assert.h>
+
+#include "guestfs.h"
+
+int
+main (int argc, char *argv[])
+{
+  guestfs_h *g = guestfs_create ();
+
+  /* Call some non-daemon functions that have a String parameter, but
+   * setting that parameter to NULL.  Previously this would cause a
+   * segfault inside libguestfs.  After this bug was fixed, this
+   * turned into an error message.
+   */
+
+  assert (guestfs_add_drive (g, NULL) == -1);
+  assert (guestfs_config (g, NULL, NULL) == -1);
+
+  /* These can be safely set to NULL, should be no error. */
+
+  assert (guestfs_set_path (g, NULL) == 0);
+  assert (guestfs_set_append (g, NULL) == 0);
+  assert (guestfs_set_qemu (g, NULL) == 0);
+
+  guestfs_close (g);
+  exit (0);
+}

diff --git a/src/generator.ml b/src/generator.ml
index f976d81..eb32698 100755 (executable)
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -5835,6 +5835,50 @@ check_state (guestfs_h *g, const char *caller)
 
 ";
 
+  let error_code_of = function
+    | RErr | RInt _ | RInt64 _ | RBool _ -> "-1"
+    | RConstString _ | RConstOptString _
+    | RString _ | RStringList _
+    | RStruct _ | RStructList _
+    | RHashtable _ | RBufferOut _ -> "NULL"
+  in
+
+  (* Generate code to check String-like parameters are not passed in
+   * as NULL (returning an error if they are).
+   *)
+  let check_null_strings shortname style =
+    let pr_newline = ref false in
+    List.iter (
+      function
+      (* parameters which should not be NULL *)
+      | String n
+      | Device n
+      | Pathname n
+      | Dev_or_Path n
+      | FileIn n
+      | FileOut n
+      | BufferIn n
+      | StringList n
+      | DeviceList n ->
+          pr "  if (%s == NULL) {\n" n;
+          pr "    error (g, \"%%s: %%s: parameter cannot be NULL\",\n";
+          pr "           \"%s\", \"%s\");\n" shortname n;
+          pr "    return %s;\n" (error_code_of (fst style));
+          pr "  }\n";
+          pr_newline := true
+
+      (* can be NULL *)
+      | OptString _
+
+      (* not applicable *)
+      | Bool _
+      | Int _
+      | Int64 _ -> ()
+    ) (snd style);
+
+    if !pr_newline then pr "\n";
+  in
+
   (* Generate code to generate guestfish call traces. *)
   let trace_call shortname style =
     pr "  if (guestfs__get_trace (g)) {\n";
@@ -5892,6 +5936,7 @@ check_state (guestfs_h *g, const char *caller)
       generate_prototype ~extern:false ~semicolon:false ~newline:true
         ~handle:"g" name style;
       pr "{\n";
+      check_null_strings shortname style;
       trace_call shortname style;
       pr "  return guestfs__%s " shortname;
       generate_c_call_args ~handle:"g" style;
@@ -5904,21 +5949,12 @@ check_state (guestfs_h *g, const char *caller)
   List.iter (
     fun (shortname, style, _, _, _, _, _) ->
       let name = "guestfs_" ^ shortname in
+      let error_code = error_code_of (fst style) in
 
       (* Generate the action stub. *)
       generate_prototype ~extern:false ~semicolon:false ~newline:true
         ~handle:"g" name style;
 
-      let error_code =
-        match fst style with
-        | RErr | RInt _ | RInt64 _ | RBool _ -> "-1"
-        | RConstString _ | RConstOptString _ ->
-            failwithf "RConstString|RConstOptString cannot be used by daemon functions"
-        | RString _ | RStringList _
-        | RStruct _ | RStructList _
-        | RHashtable _ | RBufferOut _ ->
-            "NULL" in
-
       pr "{\n";
 
       (match snd style with
@@ -5943,6 +5979,7 @@ check_state (guestfs_h *g, const char *caller)
       pr "  int serial;\n";
       pr "  int r;\n";
       pr "\n";
+      check_null_strings shortname style;
       trace_call shortname style;
       pr "  if (check_state (g, \"%s\") == -1) return %s;\n"
         shortname error_code;