Add unit tests for msnprintf().

move responsibility for adding newline if not present from
  format_errmsg() to addto_syslog() so that msnprintf() does
  not also force a trailing newline.
Correct corner case of "%\0" in msyslog() and friends.

bk: 4d104ff9XBqypLiQRNY9k627fqxOpA

diff --git a/ChangeLog b/ChangeLog
index a987d9fd7dd38a4539d9a0adcf96d63ae462e159..d4dc1756eff81496cbc78eb76a206d3c53cf3118 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,4 @@
+* Add unit tests for msnprintf().
 (4.2.7p98) 2010/12/20 Released by Harlan Stenn <stenn@ntp.org>
 * [Bug 1761] clockstuff/clktest-opts.h omitted from tarball.
 * [Bug 1762] from 4.2.6p3-RC12: manycastclient responses interfere.

diff --git a/libntp/msyslog.c b/libntp/msyslog.c
index 52f1061df2df69dd59b43d55bc953fb5e4e4a2b4..25daa23272252cae0a03fda4833e401fdaa6f863 100644 (file)
--- a/libntp/msyslog.c
+++ b/libntp/msyslog.c
@@ -37,26 +37,29 @@ extern      char *  progname;
 
 /* Declare the local functions */
 void   addto_syslog    (int, const char *);
-void   format_errmsg   (char *, int, const char *, int);
+void   format_errmsg   (char *, size_t, const char *, int);
 
 
 /*
- * This routine adds the contents of a buffer to the log
+ * This routine adds the contents of a buffer to the syslog or an
+ * application-specific logfile.
  */
 void
 addto_syslog(
        int             level,
-       const char *    buf
+       const char *    msg
        )
 {
        static char *   prevcall_progname;
        static char *   prog;
+       const char      nl[] = "\n";
+       const char      empty[] = "";
        FILE *          term_file;
        int             log_to_term;
+       int             log_to_file;
+       const char *    nl_or_empty;
        const char *    human_time;
 
-       human_time = humanlogtime();
-       log_to_term = msyslog_term;
        /* setup program basename static var prog if needed */
        if (progname != prevcall_progname) {
                prevcall_progname = progname;
@@ -67,52 +70,75 @@ addto_syslog(
                        prog = progname;
        }
 
+       log_to_term = msyslog_term;
+       log_to_file = FALSE;
 #if !defined(VMS) && !defined(SYS_VXWORKS)
        if (syslogit)
-               syslog(level, "%s", buf);
+               syslog(level, "%s", msg);
        else
-#endif /* VMS  && SYS_VXWORKS*/
-       if (syslog_file != NULL) {
-               /* syslog() provides the timestamp, program, and pid */
-               fprintf(syslog_file, "%s %s[%d]: %s", human_time, prog,
-                       (int)getpid(), buf);
-               fflush(syslog_file);
-       } else {
-               log_to_term = TRUE;
-       }
+#endif
+               if (syslog_file != NULL)
+                       log_to_file = TRUE;
+               else
+                       log_to_term = TRUE;
 #if DEBUG
        if (debug > 0)
                log_to_term = TRUE;
 #endif
+       if (!(log_to_file || log_to_term))
+               return;
+
+       /* syslog() adds the timestamp, name, and pid */
+       human_time = humanlogtime();
+
+       /* syslog() adds trailing \n if not present */
+       if ('\n' != msg[strlen(msg) - 1])
+               nl_or_empty = nl;
+       else
+               nl_or_empty = empty;
+
        if (log_to_term) {
                term_file = (level <= LOG_ERR)
                                ? stderr
                                : stdout;
-               fprintf(term_file, "%s %s[%d]: %s", human_time, prog,
-                       (int)getpid(), buf);
+               fprintf(term_file, "%s %s[%d]: %s%s", human_time, prog,
+                       (int)getpid(), msg, nl_or_empty);
                fflush(term_file);
        }
+
+       if (log_to_file) {
+               fprintf(syslog_file, "%s %s[%d]: %s%s", human_time,
+                       prog, (int)getpid(), msg, nl_or_empty);
+               fflush(syslog_file);
+       }
 }
 
 
 void
-format_errmsg(char *nfmt, int lennfmt, const char *fmt, int errval)
+format_errmsg(
+       char *          nfmt,
+       size_t          lennfmt,
+       const char *    fmt,
+       int             errval
+       )
 {
-       register char c;
-       register char *n;
-       register const char *f;
+       char c;
+       char *n;
+       const char *f;
        size_t len;
        char *err;
 
        n = nfmt;
        f = fmt;
-       while ((c = *f++) != '\0' && n < (nfmt + lennfmt - 2)) {
+       while ((c = *f++) != '\0' && n < (nfmt + lennfmt - 1)) {
                if (c != '%') {
                        *n++ = c;
                        continue;
                }
                if ((c = *f++) != 'm') {
                        *n++ = '%';
+                       if ('\0' == c)
+                               break;
                        *n++ = c;
                        continue;
                }
@@ -120,17 +146,11 @@ format_errmsg(char *nfmt, int lennfmt, const char *fmt, int errval)
                len = strlen(err);
 
                /* Make sure we have enough space for the error message */
-               if ((n + len) < (nfmt + lennfmt - 2)) {
+               if ((n + len) < (nfmt + lennfmt - 1)) {
                        memcpy(n, err, len);
                        n += len;
                }
        }
-       /*
-        * syslog adds a trailing \n if not present, do the same so we
-        * have the same behavior with syslog and a log file.
-        */
-       if (n > nfmt && '\n' != *(n - 1))
-               *n++ = '\n';
        *n = '\0';
 }
 

diff --git a/tests/libntp/Makefile.am b/tests/libntp/Makefile.am
index de9d5cfeafff887f7efece994f208d6625ade2d6..16972c58267f4816cba2485f543fbe1180be9e00 100644 (file)
--- a/tests/libntp/Makefile.am
+++ b/tests/libntp/Makefile.am
@@ -21,6 +21,7 @@ tests_SOURCES = $(top_srcdir)/sntp/tests_main.cpp     \
                inttoa.cpp              \
                lfptostr.cpp            \
                modetoa.cpp             \
+               msyslog.cpp             \
                netof.cpp               \
                numtoa.cpp              \
                numtohost.cpp           \

diff --git a/tests/libntp/msyslog.cpp b/tests/libntp/msyslog.cpp
new file mode 100644 (file)
index 0000000..491ad5a
--- /dev/null
+++ b/tests/libntp/msyslog.cpp
@@ -0,0 +1,84 @@
+#include "libntptest.h"
+
+extern "C" {
+#include <stdio.h>
+#include <errno.h>
+// format_errmsg() is normally private to msyslog.c
+void   format_errmsg   (char *, size_t, const char *, int);
+};
+
+class msyslogTest : public libntptest {
+};
+
+// msnprintf()
+TEST_F(msyslogTest, msnprintf)
+{
+#define FMT_PREFIX "msyslog.cpp ENOENT: "
+       char exp_buf[512];
+       char act_buf[512];
+
+       snprintf(exp_buf, sizeof(exp_buf), FMT_PREFIX "%s",
+                strerror(ENOENT));
+       errno = ENOENT;
+       msnprintf(act_buf, sizeof(act_buf), FMT_PREFIX "%m");
+       EXPECT_STREQ(exp_buf, act_buf);
+}
+
+TEST_F(msyslogTest, msnprintfLiteralPercentm)
+{
+       char exp_buf[32];
+       char act_buf[32];
+
+       snprintf(exp_buf, sizeof(exp_buf), "%%m");
+       errno = ENOENT;
+       msnprintf(act_buf, sizeof(act_buf), "%%m");
+       EXPECT_STREQ(exp_buf, act_buf);
+}
+
+TEST_F(msyslogTest, msnprintfBackslashLiteralPercentm)
+{
+       char exp_buf[32];
+       char act_buf[32];
+
+       snprintf(exp_buf, sizeof(exp_buf), "\%%m");
+       errno = ENOENT;
+       msnprintf(act_buf, sizeof(act_buf), "\%%m");
+       EXPECT_STREQ(exp_buf, act_buf);
+}
+
+TEST_F(msyslogTest, msnprintfBackslashPercent)
+{
+       char exp_buf[32];
+       char act_buf[32];
+
+       snprintf(exp_buf, sizeof(exp_buf), "\%s", strerror(ENOENT));
+       errno = ENOENT;
+       msnprintf(act_buf, sizeof(act_buf), "\%m");
+       EXPECT_STREQ(exp_buf, act_buf);
+}
+
+TEST_F(msyslogTest, msnprintfHangingPercent)
+{
+       static char fmt[] = "percent then nul term then non-nul %\0oops!";
+       char exp_buf[64];
+       char act_buf[64];
+
+       memset(exp_buf, 0, sizeof(exp_buf));
+       memset(act_buf, 0, sizeof(act_buf));
+       snprintf(exp_buf, sizeof(exp_buf), fmt);
+       msnprintf(act_buf, sizeof(act_buf), fmt);
+       EXPECT_STREQ(exp_buf, act_buf);
+       EXPECT_STREQ("", act_buf + 1 + strlen(act_buf));
+}
+
+TEST_F(msyslogTest, format_errmsgHangingPercent)
+{
+       static char fmt[] = "percent then nul term then non-nul %\0oops!";
+       char act_buf[64];
+
+       memset(act_buf, 0, sizeof(act_buf));
+       format_errmsg(act_buf, sizeof(act_buf), fmt, ENOENT);
+       EXPECT_STREQ(fmt, act_buf);
+       EXPECT_STREQ("", act_buf + 1 + strlen(act_buf));
+}
+