libcdecl: reduce snprintf reliance for error reporting.

Add a fallback formatting method using the main libcdecl rendering
functions which works in the NLS-disabled case.

We don't need to avoid snprintf if NLS is enabled, as GNU libintl
will provide a usable snprintf if the system does not.

diff --git a/Makefile.am b/Makefile.am
index 4af8e9a01c3febd3fe7f6f790a6ba260e921f308..336e4439ba82f760cb776d09d7ef32fb8894cafd 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -115,7 +115,7 @@ t/cdeclerr.$(OBJEXT): src/errmsg.h
 
 check_PROGRAMS += t/cdeclerr
 t_cdeclerr_SOURCES = common/src/tap.c t/cdeclerr.c
-EXTRA_t_cdeclerr_DEPENDENCIES = src/error.lo $(shared_gl_objects)
+EXTRA_t_cdeclerr_DEPENDENCIES = src/error.lo src/output.lo $(shared_gl_objects)
 t_cdeclerr_LDADD = $(EXTRA_t_cdeclerr_DEPENDENCIES) $(LIBTHREAD)
 $(t_cdeclerr_OBJECTS): $(gnulib_headers)
 

diff --git a/src/cdecl-internal.h b/src/cdecl-internal.h
index 186c6b04cfd9460508e477d988b56344cc2c53cd..a1e8a9665b21ebe9593dca36eb25142e35648abe 100644 (file)
--- a/src/cdecl-internal.h
+++ b/src/cdecl-internal.h
@@ -22,6 +22,8 @@
 #include <gettext.h>
 #include "cdecl.h"
 
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+
 #define _(s) dgettext(PACKAGE, s)
 #define N_(s) s
 

diff --git a/src/error.c b/src/error.c
index cf4411e0e89b8da7dfbe82fcbda7ddf0c18c871b..758f58e63c50d4159195b08189c10cd6467e06c2 100644 (file)
--- a/src/error.c
+++ b/src/error.c
@@ -134,6 +134,35 @@ void cdecl__errmsg(unsigned msg)
        set_err(msg, &state->err);
 }
 
+/*
+ * In the NLS-disabled case, all format strings are of the form
+ *
+ *   "blah blah %s"
+ *
+ * so we exploit this to implement a simple snprintf workalike using the
+ * libcdecl output helpers directly.
+ *
+ * In the NLS-enabled case, we have to use snprintf as format strings may
+ * be translated.  GNU libintl ensures a suitable version is available.
+ */
+static size_t
+fmt_err(struct err_state *state, const char *fmt, const char *arg)
+{
+#if ENABLE_NLS
+       snprintf(state->str, state->nstr, fmt, arg);
+       return strlen(fmt) + strlen(arg);
+#else
+       struct output_state dst = { state->str, state->nstr };
+       size_t rc;
+
+       rc = cdecl__strlcpy(dst.dst, fmt, dst.dstlen);
+       cdecl__advance(&dst, rc-2);
+       cdecl__emit(&dst, arg);
+
+       return dst.accum;
+#endif
+}
+
 /*
  * Sets the library error to code; fmt is a printf-style string that may use
  * up to one %s directive, to refer to arg.
@@ -141,16 +170,18 @@ void cdecl__errmsg(unsigned msg)
 void cdecl__err(unsigned code, const char *fmt, const char *arg)
 {
        struct err_state *state;
-       int rc, try = 0;
+       unsigned try = 0;
+       size_t rc;
 
        state = get_err_state();
        if (!state)
                return;
 
 retry:
-       rc = snprintf(state->str, state->nstr, fmt, arg);
-       if (rc > 0 && rc >= state->nstr) {
+       rc = fmt_err(state, fmt, arg);
+       if (rc >= state->nstr) {
                assert(try++ == 0 && rc < SIZE_MAX / 4);
+
                state = alloc_err_state(state, (size_t)(rc+1u) * 3 / 2);
                if (!state)
                        return;

diff --git a/src/output.c b/src/output.c
index 047fc64ed087138d0d0ceb305485a54d88b0c89d..fb80f99e7128e17606498287a4b936b4b1d311dc 100644 (file)
--- a/src/output.c
+++ b/src/output.c
@@ -25,8 +25,6 @@
 #include "parse.h"
 #include "specstr.h"
 
-#define MIN(a, b) ((a) < (b) ? (a) : (b))
-
 size_t cdecl__advance(struct output_state *dst, size_t amount)
 {
        size_t x = MIN(amount, dst->dstlen);

diff --git a/t/cdeclerr.c b/t/cdeclerr.c
index b69f7e60971a8088731889df2cfa4f13ccf078d5..b4a014ca1e8b2b06fa1c93a0c526007d913eb188 100644 (file)
--- a/t/cdeclerr.c
+++ b/t/cdeclerr.c
@@ -27,6 +27,11 @@
 #include "errmsg.h"
 #include "tap.h"
 
+const char *cdecl__token_name(unsigned token)
+{
+       assert(0);
+}
+
 static char *fmt_char(int c, char *buf)
 {
        int escape = 0;
@@ -53,6 +58,15 @@ static char *fmt_char(int c, char *buf)
        return buf;
 }
 
+static void check_code(const struct cdecl_error *err, unsigned expect)
+{
+       if (!tap_result(err->code == expect, "returned error code")) {
+               tap_diag("Failed, unexpected result");
+               tap_diag("   Received: %u", err->code);
+               tap_diag("   Expected: %u", expect);
+       }
+}
+
 static void check_fixed_string(size_t len)
 {
        const struct cdecl_error *err;
@@ -69,15 +83,11 @@ static void check_fixed_string(size_t len)
        memset(work2, 'X', len - 1);
 
        tap_diag("cdecl__err w/ %lu-byte string", (unsigned long)len);
-       cdecl__err(1234, work1, "");
+       cdecl__err(1234, work1, "XX");
        memset(work1, 0, len);
        err = cdecl_get_error();
 
-       if (!tap_result(err->code == 1234, "returned error code")) {
-               tap_diag("Failed, unexpected result");
-               tap_diag("   Received: %u", err->code);
-               tap_diag("   Expected: 1234");
-       }
+       check_code(err, 1234);
 
        errlen = strlen(err->str);
        if (!tap_result(errlen == len-1, "returned string length")) {
@@ -103,6 +113,32 @@ static void check_fixed_string(size_t len)
        free(work1);
 }
 
+static void check_format_string(const char *fmt, const char *arg)
+{
+       size_t sz = strlen(fmt) + strlen(arg);
+       const struct cdecl_error *err;
+       char *work;
+
+       work = malloc(sz + 1);
+       if (!work)
+               abort();
+       sprintf(work, fmt, arg);
+
+       cdecl__err(5432, fmt, arg);
+       err = cdecl_get_error();
+
+       tap_diag("cdecl__err(\"%s\", \"%s\")", fmt, arg);
+       check_code(err, 5432);
+
+       if (!tap_result(!strcmp(err->str, work), "returned string")) {
+               tap_diag("Failed, unexpected result");
+               tap_diag("   Received: %.*s", (int)sz, err->str);
+               tap_diag("   Expected: %s", work);
+       }
+
+       free(work);
+}
+
 static void check_enomem()
 {
        const char expmsg[] = "failed to allocate memory";
@@ -112,12 +148,7 @@ static void check_enomem()
        cdecl__errmsg(CDECL__ENOMEM);
        err = cdecl_get_error();
 
-       if (!tap_result(err->code == CDECL_ENOMEM, "returned error code")) {
-               tap_diag("Failed, unexpected result");
-               tap_diag("   Received: %u", err->code);
-               tap_diag("   Expected: %d", CDECL_ENOMEM);
-       }
-
+       check_code(err, CDECL_ENOMEM);
        if (!tap_result(!strcmp(err->str, "failed to allocate memory"),
                        "returned string"))
        {
@@ -134,14 +165,23 @@ static void check_enomem()
        }
 }
 
+#define IFNLS(a, b) (ENABLE_NLS ? (a) : (b))
+
 int main(void)
 {
-       tap_plan(3*3 + 2);
+       tap_plan(3*3 + 2 + 2);
 
        check_fixed_string(50);
        check_fixed_string(500);
        check_fixed_string(5000);
 
+       /*
+        * When NLS is disabled, for implementation simplicity only format
+        * strings ending with %s are supported as this is sufficient.
+        * Otherwise, %s may appear anywhere.
+        */
+       check_format_string(IFNLS("hello %s world", "hello world %s"), "za");
+
        check_enomem();
 
        tap_done();