Issue #189: Eliminate use of MC_ERROR from json_util.c, and add a json_util_get_last_err() function to retrieve the error for those callers that care about it.

Add tests and descriptions for the functions in json_util.c

.gitignore

@@ -47,6 +47,7 @@
 /tests/test_printbuf
 /tests/test_set_serializer
 /tests/test_compare
+/tests/test_util_file
 /tests/*.vg.out
 /tests/*.log
 /tests/*.trs

json_util.c

@@ -12,6 +12,7 @@
 #include "config.h"
 #undef realloc
 
+#include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stddef.h>
@@ -63,15 +64,26 @@
 static int sscanf_is_broken = 0;
 static int sscanf_is_broken_testdone = 0;
 static void sscanf_is_broken_test(void);
+static void _set_last_err(const char *err_fmt, ...);
+
+static char _last_err[256] = "";
+
+const char *json_util_get_last_err()
+{
+	if (_last_err[0] == '\0')
+		return NULL;
+	return _last_err;
+}
+
+static void _set_last_err(const char *err_fmt, ...)
+{
+	va_list ap;
+	va_start(ap, err_fmt);
+	// Ignore (attempted) overruns from snprintf
+	(void)vsnprintf(_last_err, sizeof(_last_err), err_fmt, ap);
+	va_end(ap);
+}
 
-/*
- * Create a JSON object from already opened file descriptor.
- *
- * This function can be helpful, when you opened the file already,
- * e.g. when you have a temp file.
- * Note, that the fd must be readable at the actual position, i.e.
- * use lseek(fd, 0, SEEK_SET) before.
- */
 struct json_object* json_object_from_fd(int fd)
 {
   struct printbuf *pb;
@@ -80,14 +92,14 @@ struct json_object* json_object_from_fd(int fd)
   int ret;
 
   if(!(pb = printbuf_new())) {
-    MC_ERROR("json_object_from_file: printbuf_new failed\n");
+    _set_last_err("json_object_from_file: printbuf_new failed\n");
     return NULL;
   }
   while((ret = read(fd, buf, JSON_FILE_BUF_SIZE)) > 0) {
     printbuf_memappend(pb, buf, ret);
   }
   if(ret < 0) {
-    MC_ERROR("json_object_from_fd: error reading fd %d: %s\n", fd, strerror(errno));
+    _set_last_err("json_object_from_fd: error reading fd %d: %s\n", fd, strerror(errno));
     printbuf_free(pb);
     return NULL;
   }
@@ -102,7 +114,7 @@ struct json_object* json_object_from_file(const char *filename)
   int fd;
 
   if((fd = open(filename, O_RDONLY)) < 0) {
-    MC_ERROR("json_object_from_file: error opening file %s: %s\n",
+    _set_last_err("json_object_from_file: error opening file %s: %s\n",
 	     filename, strerror(errno));
     return NULL;
   }
@@ -120,12 +132,12 @@ int json_object_to_file_ext(const char *filename, struct json_object *obj, int f
   unsigned int wpos, wsize;
 
   if(!obj) {
-    MC_ERROR("json_object_to_file: object is null\n");
+    _set_last_err("json_object_to_file: object is null\n");
     return -1;
   }
 
   if((fd = open(filename, O_WRONLY | O_TRUNC | O_CREAT, 0644)) < 0) {
-    MC_ERROR("json_object_to_file: error opening file %s: %s\n",
+    _set_last_err("json_object_to_file: error opening file %s: %s\n",
 	     filename, strerror(errno));
     return -1;
   }
@@ -140,7 +152,7 @@ int json_object_to_file_ext(const char *filename, struct json_object *obj, int f
   while(wpos < wsize) {
     if((ret = write(fd, json_str + wpos, wsize-wpos)) < 0) {
       close(fd);
-      MC_ERROR("json_object_to_file: error writing file %s: %s\n",
+      _set_last_err("json_object_to_file: error writing file %s: %s\n",
 	     filename, strerror(errno));
       return -1;
     }
@@ -306,7 +318,7 @@ const char *json_type_to_name(enum json_type o_type)
 	int o_type_int = (int)o_type;
 	if (o_type_int < 0 || o_type_int >= (int)NELEM(json_type_name))
 	{
-		MC_ERROR("json_type_to_name: type %d is out of range [0,%d]\n", o_type, NELEM(json_type_name));
+		_set_last_err("json_type_to_name: type %d is out of range [0,%d]\n", o_type, NELEM(json_type_name));
 		return NULL;
 	}
 	return json_type_name[o_type];

json_util.h

@@ -30,14 +30,52 @@ extern "C" {
 #define JSON_FILE_BUF_SIZE 4096
 
 /* utility functions */
+/**
+ * Read the full contents of the given file, then convert it to a
+ * json_object using json_tokener_parse().
+ *
+ * Returns -1 if something fails.  See json_util_get_last_err() for details.
+ */
 extern struct json_object* json_object_from_file(const char *filename);
+
+/**
+ * Create a JSON object from already opened file descriptor.
+ *
+ * This function can be helpful, when you opened the file already,
+ * e.g. when you have a temp file.
+ * Note, that the fd must be readable at the actual position, i.e.
+ * use lseek(fd, 0, SEEK_SET) before.
+ *
+ * Returns -1 if something fails.  See json_util_get_last_err() for details.
+ */
 extern struct json_object* json_object_from_fd(int fd);
+
+/**
+ * Equivalent to:
+ *   json_object_to_file_ext(filename, obj, JSON_C_TO_STRING_PLAIN);
+ *
+ * Returns -1 if something fails.  See json_util_get_last_err() for details.
+ */
 extern int json_object_to_file(const char *filename, struct json_object *obj);
+
+/**
+ * Open and truncate the given file, creating it if necessary, then
+ * convert the json_object to a string and write it to the file.
+ *
+ * Returns -1 if something fails.  See json_util_get_last_err() for details.
+ */
 extern int json_object_to_file_ext(const char *filename, struct json_object *obj, int flags);
+
+/**
+ * Return the last error from json_object_to_file{,_ext} or
+ * json_object_from_{file,fd}, or NULL if there is none.
+ */
+const char *json_util_get_last_err(void);
+
+
 extern int json_parse_int64(const char *buf, int64_t *retval);
 extern int json_parse_double(const char *buf, double *retval);
 
-
 /**
  * Return a string describing the type of the object.
  * e.g. "int", or "object", etc...

tests/Makefile.am

@@ -4,6 +4,7 @@ LDADD= $(LIBJSON_LA)
 LIBJSON_LA=$(top_builddir)/libjson-c.la
 
 TESTS=
+TESTS+= test_util_file.test
 TESTS+= test1.test
 TESTS+= test2.test
 TESTS+= test4.test
@@ -32,6 +33,8 @@ check_PROGRAMS += test2Formatted
 test2Formatted_SOURCES = test2.c parse_flags.c
 test2Formatted_CPPFLAGS = -DTEST_FORMATTED
 
+test_util_file_SOURCES = test_util_file.c strerror_override.c
+
 EXTRA_DIST=
 EXTRA_DIST += $(TESTS)
 

tests/strerror_override.c

@@ -0,0 +1,90 @@
+#include <errno.h>
+
+/*
+ * Override strerror() to get consistent output across platforms.
+ */
+
+static struct {
+	int errno_value;
+	const char *errno_str;
+} errno_list[] = {
+#define STRINGIFY(x) #x
+#define ENTRY(x) {x, &STRINGIFY(undef_ ## x)[6]}
+	ENTRY(EPERM),
+	ENTRY(ENOENT),
+	ENTRY(ESRCH),
+	ENTRY(EINTR),
+	ENTRY(EIO),
+	ENTRY(ENXIO),
+	ENTRY(E2BIG),
+	ENTRY(ENOEXEC),
+	ENTRY(EBADF),
+	ENTRY(ECHILD),
+	ENTRY(EDEADLK),
+	ENTRY(ENOMEM),
+	ENTRY(EACCES),
+	ENTRY(EFAULT),
+	ENTRY(ENOTBLK),
+	ENTRY(EBUSY),
+	ENTRY(EEXIST),
+	ENTRY(EXDEV),
+	ENTRY(ENODEV),
+	ENTRY(ENOTDIR),
+	ENTRY(EISDIR),
+	ENTRY(EINVAL),
+	ENTRY(ENFILE),
+	ENTRY(EMFILE),
+	ENTRY(ENOTTY),
+	ENTRY(ETXTBSY),
+	ENTRY(EFBIG),
+	ENTRY(ENOSPC),
+	ENTRY(ESPIPE),
+	ENTRY(EROFS),
+	ENTRY(EMLINK),
+	ENTRY(EPIPE),
+	ENTRY(EDOM),
+	ENTRY(ERANGE),
+	ENTRY(EAGAIN),
+	{ 0, (char *)0 }
+};
+
+#define PREFIX "ERRNO="
+static char errno_buf[128] = PREFIX;
+const char *strerror(int errno_in)
+{
+	int start_idx;
+	char digbuf[20];
+	int ii, jj;
+
+	// Avoid standard functions, so we don't need to include any
+	// headers, or guess at signatures.
+
+	for (ii = 0; errno_list[ii].errno_str != (char *)0; ii++)
+	{
+		const char *errno_str = errno_list[ii].errno_str;
+		if (errno_list[ii].errno_value != errno_in)
+			continue;
+
+		for (start_idx = sizeof(PREFIX) - 1, jj = 0; errno_str[jj] != '\0'; jj++, start_idx++)
+		{
+			errno_buf[start_idx] = errno_str[jj];
+		}
+		errno_buf[start_idx] = '\0';
+		return errno_buf;
+	}
+
+	// It's not one of the known errno values, return the numeric value.
+	for (ii = 0; errno_in > 10; errno_in /= 10, ii++)
+	{
+		digbuf[ii] = "0123456789"[(errno_in % 10)];
+	}
+	digbuf[ii] = "0123456789"[(errno_in % 10)];
+
+	// Reverse the digits
+	for (start_idx = sizeof(PREFIX) - 1 ; ii >= 0; ii--, start_idx++)
+	{
+		errno_buf[start_idx] = digbuf[ii];
+	}
+	return errno_buf;
+}
+

tests/test_util_file.c

@@ -0,0 +1,155 @@
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <stddef.h>
+#include <string.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+#include "json.h"
+#include "json_util.h"
+
+static void test_read_valid_with_fd(const char *testdir);
+static void test_read_nonexistant(const char *testdir);
+static void test_read_closed(void);
+
+static void test_write_to_file();
+static void stat_and_cat(const char *file);
+
+static void test_write_to_file()
+{
+	json_object *jso;
+
+	jso = json_tokener_parse("{"
+		"\"foo\":1234,"
+		"\"foo1\":\"abcdefghijklmnopqrstuvwxyz\","
+		"\"foo2\":\"abcdefghijklmnopqrstuvwxyz\","
+		"\"foo3\":\"abcdefghijklmnopqrstuvwxyz\","
+		"\"foo4\":\"abcdefghijklmnopqrstuvwxyz\","
+		"\"foo5\":\"abcdefghijklmnopqrstuvwxyz\","
+		"\"foo6\":\"abcdefghijklmnopqrstuvwxyz\","
+		"\"foo7\":\"abcdefghijklmnopqrstuvwxyz\","
+		"\"foo8\":\"abcdefghijklmnopqrstuvwxyz\","
+		"\"foo9\":\"abcdefghijklmnopqrstuvwxyz\""
+		"}");
+	const char *outfile = "json.out";
+	int rv = json_object_to_file(outfile, jso);
+	printf("%s: json_object_to_file(%s, jso)=%d\n",
+		(rv == 0) ? "OK" : "FAIL", outfile, rv);
+	if (rv == 0)
+		stat_and_cat(outfile);
+
+	printf("\n");
+
+	const char *outfile2 = "json2.out";
+	rv = json_object_to_file_ext(outfile2, jso, JSON_C_TO_STRING_PRETTY);
+	printf("%s: json_object_to_file_ext(%s, jso, JSON_C_TO_STRING_PRETTY)=%d\n",
+	     (rv == 0) ? "OK" : "FAIL", outfile2, rv);
+	if (rv == 0)
+		stat_and_cat(outfile2);
+}
+
+static void stat_and_cat(const char *file)
+{
+	struct stat sb;
+	int d = open(file, O_RDONLY, 0600);
+	if (d < 0)
+	{
+		printf("FAIL: unable to open %s: %s\n", file, strerror(errno));
+		return;
+	}
+	if (fstat(d, &sb) < 0)
+	{
+		printf("FAIL: unable to stat %s: %s\n", file, strerror(errno));
+		close(d);
+		return;
+	}
+	char *buf = malloc(sb.st_size + 1);
+	if (read(d, buf, sb.st_size) < sb.st_size)
+	{
+		printf("FAIL: unable to read all of %s: %s\n", file, strerror(errno));
+		close(d);
+		return;
+	}
+	buf[sb.st_size] = '\0';
+	printf("file[%s], size=%d, contents=%s\n", file, (int)sb.st_size, buf);
+}
+
+int main(int argc, char **argv)
+{
+//	json_object_to_file(file, obj);
+//	json_object_to_file_ext(file, obj, flags);
+
+	const char *testdir;
+	if (argc < 2)
+	{
+		fprintf(stderr, "Usage: %s <testdir>\n  <testdir> is the location of input files\n", argv[0]);
+		exit(1);
+	}
+	testdir = argv[1];
+
+	test_read_valid_with_fd(testdir);
+	test_read_nonexistant(testdir);
+	test_read_closed();
+	test_write_to_file();
+}
+
+static void test_read_valid_with_fd(const char *testdir)
+{
+	char file_buf[4096];
+	(void)snprintf(file_buf, sizeof(file_buf), "%s/valid.json", testdir);
+
+	int d = open(file_buf, O_RDONLY, 0);
+	if (d < 0)
+	{
+		fprintf(stderr, "FAIL: unable to open %s: %s\n", file_buf, strerror(errno));
+		exit(1);
+	}
+	json_object *jso = json_object_from_fd(d);
+	if (jso != NULL)
+	{
+		printf("OK: json_object_from_fd(%s)=%s\n", file_buf, json_object_to_json_string(jso));
+		json_object_put(jso);
+	}
+	else
+	{
+		fprintf(stderr, "FAIL: unable to parse contents of %s: %s\n", file_buf, json_util_get_last_err());
+	}
+	close(d);
+}
+
+static void test_read_nonexistant(const char *testdir)
+{
+	char file_buf[4096];
+	const char *filename = "not_present.json";
+	(void)snprintf(file_buf, sizeof(file_buf), "%s/%s", testdir, filename);
+
+	json_object *jso = json_object_from_file(file_buf);
+	if (jso != NULL)
+	{
+		printf("FAIL: json_object_from_file(%s) returned %p when NULL expected\n", filename, jso);
+		json_object_put(jso);
+	}
+	else
+	{
+		printf("OK: json_object_from_file(%s) correctly returned NULL: %s\n",
+		       filename, json_util_get_last_err());
+	}
+}
+
+static void test_read_closed()
+{
+	// Test reading from a closed fd
+	int d = open("/dev/null", O_RDONLY, 0);
+	close(d);
+	json_object *jso = json_object_from_fd(d);
+	if (jso != NULL)
+	{
+		printf("FAIL: read from closed fd returning non-NULL: %p\n", jso);
+		fflush(stdout);
+		printf("  jso=%s\n", json_object_to_json_string(jso));
+		json_object_put(jso);
+		return;
+	}
+	printf("OK: json_object_from_fd(closed_fd), expecting NULL, EBADF, got:%p, %s\n", jso, json_util_get_last_err());
+}

tests/test_util_file.expected

@@ -0,0 +1,21 @@
+OK: json_object_from_fd(/home/erh/json-c/json-c/tests/valid.json)={ "foo": 123 }
+OK: json_object_from_file(not_present.json) correctly returned NULL: json_object_from_file: error opening file /home/erh/json-c/json-c/tests/not_present.json: ERRNO=ENOENT
+
+OK: json_object_from_fd(closed_fd), expecting NULL, EBADF, got:0x0, json_object_from_fd: error reading fd 3: ERRNO=EBADF
+
+OK: json_object_to_file(json.out, jso)=0
+file[json.out], size=336, contents={"foo":1234,"foo1":"abcdefghijklmnopqrstuvwxyz","foo2":"abcdefghijklmnopqrstuvwxyz","foo3":"abcdefghijklmnopqrstuvwxyz","foo4":"abcdefghijklmnopqrstuvwxyz","foo5":"abcdefghijklmnopqrstuvwxyz","foo6":"abcdefghijklmnopqrstuvwxyz","foo7":"abcdefghijklmnopqrstuvwxyz","foo8":"abcdefghijklmnopqrstuvwxyz","foo9":"abcdefghijklmnopqrstuvwxyz"}
+
+OK: json_object_to_file_ext(json2.out, jso, JSON_C_TO_STRING_PRETTY)=0
+file[json2.out], size=367, contents={
+  "foo":1234,
+  "foo1":"abcdefghijklmnopqrstuvwxyz",
+  "foo2":"abcdefghijklmnopqrstuvwxyz",
+  "foo3":"abcdefghijklmnopqrstuvwxyz",
+  "foo4":"abcdefghijklmnopqrstuvwxyz",
+  "foo5":"abcdefghijklmnopqrstuvwxyz",
+  "foo6":"abcdefghijklmnopqrstuvwxyz",
+  "foo7":"abcdefghijklmnopqrstuvwxyz",
+  "foo8":"abcdefghijklmnopqrstuvwxyz",
+  "foo9":"abcdefghijklmnopqrstuvwxyz"
+}

tests/test_util_file.test

@@ -0,0 +1,14 @@
+#!/bin/sh
+
+# Common definitions
+if test -z "$srcdir"; then
+    srcdir="${0%/*}"
+    test "$srcdir" = "$0" && srcdir=.
+    test -z "$srcdir" && srcdir=.
+fi
+. "$srcdir/test-defs.sh"
+
+run_output_test test_util_file "$srcdir"
+_err=$?
+
+exit $_err

tests/valid.json

@@ -0,0 +1 @@
+{"foo":123}