From 29ef73f21d67d0ee9c2b15f0749876dc0bc983ad Mon Sep 17 00:00:00 2001 From: Eric Haszlakiewicz Date: Sun, 26 Jun 2016 02:20:05 +0000 Subject: [PATCH] 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 | 1 + json_util.c | 42 +++++---- json_util.h | 40 ++++++++- tests/Makefile.am | 3 + tests/strerror_override.c | 90 ++++++++++++++++++++ tests/test_util_file.c | 155 ++++++++++++++++++++++++++++++++++ tests/test_util_file.expected | 21 +++++ tests/test_util_file.test | 14 +++ tests/valid.json | 1 + 9 files changed, 351 insertions(+), 16 deletions(-) create mode 100644 tests/strerror_override.c create mode 100644 tests/test_util_file.c create mode 100644 tests/test_util_file.expected create mode 100755 tests/test_util_file.test create mode 100644 tests/valid.json diff --git a/.gitignore b/.gitignore index 6e0bee23a3..ef02d5628c 100644 --- a/.gitignore +++ b/.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 diff --git a/json_util.c b/json_util.c index 910d84ceca..b66c3dc310 100644 --- a/json_util.c +++ b/json_util.c @@ -12,6 +12,7 @@ #include "config.h" #undef realloc +#include #include #include #include @@ -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]; diff --git a/json_util.h b/json_util.h index f6fc52cff6..a9132787bb 100644 --- a/json_util.h +++ b/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... diff --git a/tests/Makefile.am b/tests/Makefile.am index 7fdb2d9236..eb1364bf94 100644 --- a/tests/Makefile.am +++ b/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) diff --git a/tests/strerror_override.c b/tests/strerror_override.c new file mode 100644 index 0000000000..a5c2292565 --- /dev/null +++ b/tests/strerror_override.c @@ -0,0 +1,90 @@ +#include + +/* + * 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; +} + diff --git a/tests/test_util_file.c b/tests/test_util_file.c new file mode 100644 index 0000000000..23c858beae --- /dev/null +++ b/tests/test_util_file.c @@ -0,0 +1,155 @@ +#include +#include +#include +#include +#include +#include +#include + +#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 \n 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()); +} diff --git a/tests/test_util_file.expected b/tests/test_util_file.expected new file mode 100644 index 0000000000..5df22d6ba7 --- /dev/null +++ b/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" +} diff --git a/tests/test_util_file.test b/tests/test_util_file.test new file mode 100755 index 0000000000..b293523bda --- /dev/null +++ b/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 diff --git a/tests/valid.json b/tests/valid.json new file mode 100644 index 0000000000..bde58e7952 --- /dev/null +++ b/tests/valid.json @@ -0,0 +1 @@ +{"foo":123}