[PATCH 0/8] pull: logger: add tests and fix couple issues

[PATCH 0/8] pull: logger: add tests and fix couple issues

[Sami Kerola]

Hello,

Coverage of these tests is not magnificent.  None of the communications
to external systems using tcp, udp, or to journald are tested, but at
least some aspects of the logger(1) functionality gets to be checked -
that is message formats and input validation.


----------------------------------------------------------------
The following changes since commit 7ff6948e59172c1942544c9a064708e5666ba20d:
  logger: use xstrdup() (2015-03-13 15:36:39 +0100)
are available in the git repository at:
   tests_logger 
for you to fetch changes up to e3f7951113a68b171b5cea52ae0d36609faf4438:
  tests: add logger(1) error condition tests (2015-03-15 12:46:30 +0000)
----------------------------------------------------------------

Sami Kerola (8):
  logger: tidy few indentation issues
  logger: check xgethostname() return value
  tests: add test_logger helper command
  tests: add logger(1) command line options tests
  logger: fix rfc5424 format crash
  tests: add logger(1) message format tests
  logger: use errx() when checking user input
  tests: add logger(1) error condition tests

 misc-utils/Makemodule.am                           |   4 +
 misc-utils/logger.c                                | 173 +++++++++++++--------
 tests/commands.sh                                  |   1 +
 tests/expected/logger/errors                       |   0
 tests/expected/logger/errors-id_with_space         |   2 +
 tests/expected/logger/errors-invalid_prio          |   1 +
 tests/expected/logger/errors-kern_priority         |   1 +
 tests/expected/logger/errors-kern_priority_numeric |   1 +
 tests/expected/logger/errors-rfc5424_exceed_size   |   1 +
 .../logger/errors-rfc5424_msgid_with_space         |   1 +
 tests/expected/logger/errors-tag_with_space        |   2 +
 tests/expected/logger/formats                      |   0
 tests/expected/logger/formats-priorities           | 152 ++++++++++++++++++
 tests/expected/logger/formats-rfc3164              |   1 +
 tests/expected/logger/formats-rfc5424_msgid        |   1 +
 tests/expected/logger/formats-rfc5424_nohost       |   1 +
 tests/expected/logger/formats-rfc5424_notime       |   1 +
 tests/expected/logger/formats-rfc5424_simple       |   1 +
 tests/expected/logger/input_empty_line             |   3 +
 tests/expected/logger/input_prio_prefix            |   1 +
 tests/expected/logger/input_simple                 |   1 +
 tests/expected/logger/options                      |   0
 .../expected/logger/options-input_file_empty_line  |   3 +
 .../expected/logger/options-input_file_prio_prefix |   1 +
 tests/expected/logger/options-input_file_simple    |   1 +
 .../expected/logger/options-input_file_skip_empty  |   2 +
 tests/expected/logger/options-log_pid              |   1 +
 tests/expected/logger/options-log_pid_define       |   1 +
 tests/expected/logger/options-log_pid_long         |   1 +
 tests/expected/logger/options-log_pid_no_arg       |   1 +
 tests/expected/logger/options-simple               |   1 +
 tests/ts/logger/errors                             |  60 +++++++
 tests/ts/logger/formats                            |  53 +++++++
 tests/ts/logger/options                            |  57 +++++++
 34 files changed, 465 insertions(+), 66 deletions(-)
 create mode 100644 tests/expected/logger/errors
 create mode 100644 tests/expected/logger/errors-id_with_space
 create mode 100644 tests/expected/logger/errors-invalid_prio
 create mode 100644 tests/expected/logger/errors-kern_priority
 create mode 100644 tests/expected/logger/errors-kern_priority_numeric
 create mode 100644 tests/expected/logger/errors-rfc5424_exceed_size
 create mode 100644 tests/expected/logger/errors-rfc5424_msgid_with_space
 create mode 100644 tests/expected/logger/errors-tag_with_space
 create mode 100644 tests/expected/logger/formats
 create mode 100644 tests/expected/logger/formats-priorities
 create mode 100644 tests/expected/logger/formats-rfc3164
 create mode 100644 tests/expected/logger/formats-rfc5424_msgid
 create mode 100644 tests/expected/logger/formats-rfc5424_nohost
 create mode 100644 tests/expected/logger/formats-rfc5424_notime
 create mode 100644 tests/expected/logger/formats-rfc5424_simple
 create mode 100644 tests/expected/logger/input_empty_line
 create mode 100644 tests/expected/logger/input_prio_prefix
 create mode 100644 tests/expected/logger/input_simple
 create mode 100644 tests/expected/logger/options
 create mode 100644 tests/expected/logger/options-input_file_empty_line
 create mode 100644 tests/expected/logger/options-input_file_prio_prefix
 create mode 100644 tests/expected/logger/options-input_file_simple
 create mode 100644 tests/expected/logger/options-input_file_skip_empty
 create mode 100644 tests/expected/logger/options-log_pid
 create mode 100644 tests/expected/logger/options-log_pid_define
 create mode 100644 tests/expected/logger/options-log_pid_long
 create mode 100644 tests/expected/logger/options-log_pid_no_arg
 create mode 100644 tests/expected/logger/options-simple
 create mode 100755 tests/ts/logger/errors
 create mode 100755 tests/ts/logger/formats
 create mode 100755 tests/ts/logger/options

-- 
2.3.2

[PATCH 1/8] logger: tidy few indentation issues

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/logger.c | 115 ++++++++++++++++++++++++++--------------------------
 1 file changed, 58 insertions(+), 57 deletions(-)

diff --git a/misc-utils/logger.c b/misc-utils/logger.c
index ab734dd..e3fc248 100644
--- a/misc-utils/logger.c
+++ b/misc-utils/logger.c
@@ -159,8 +159,8 @@ static int pencode(char *s)
 	level = decode(s, prioritynames);
 	if (level < 0)
 		errx(EXIT_FAILURE, _("unknown priority name: %s"), s);
-	if(facility == LOG_KERN)
-		facility = LOG_USER; /* kern is forbidden */
+	if (facility == LOG_KERN)
+		facility = LOG_USER;	/* kern is forbidden */
 	return ((level & LOG_PRIMASK) | (facility & LOG_FACMASK));
 }
 
@@ -294,21 +294,21 @@ static char *xgetlogin(void)
 	return cp;
 }
 
-
 /* this creates a timestamp based on current time according to the
  * fine rules of RFC3164, most importantly it ensures in a portable
  * way that the month day is correctly written (with a SP instead
  * of a leading 0). The function uses a static buffer which is
  * overwritten on the next call (just like ctime() does).
  */
-static const char *
-rfc3164_current_time(void)
+static const char *rfc3164_current_time(void)
 {
 	static char time[32];
 	struct timeval tv;
 	struct tm *tm;
-	static char *monthnames[] = { "Jan", "Feb", "Mar", "Apr",
-			"May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec"};
+	static char *monthnames[] = {
+		"Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug",
+		"Sep", "Oct", "Nov", "Dec"
+	};
 
 	gettimeofday(&tv, NULL);
 	tm = localtime(&tv.tv_sec);
@@ -329,16 +329,15 @@ static void write_output(const struct logger_ctl *ctl, const char *const msg)
 	const size_t len = xasprintf(&buf, "%s%s", ctl->hdr, msg);
 	if (write_all(ctl->fd, buf, len) < 0)
 		warn(_("write failed"));
-	else
-		if (ctl->socket_type == TYPE_TCP)
-			/* using an additional write seems like the best compromise:
-			 * - writev() is not yet supported by framework
-			 * - adding the \n to the buffer in formatters violates layers
-			 * - adding \n after the fact requires memory copy
-			 * - logger is not a high-performance app
-			 */
-			if (write_all(ctl->fd, "\n", 1) < 0)
-				warn(_("write failed"));
+	else if (ctl->socket_type == TYPE_TCP)
+		/* using an additional write seems like the best compromise:
+		 * - writev() is not yet supported by framework
+		 * - adding the \n to the buffer in formatters violates layers
+		 * - adding \n after the fact requires memory copy
+		 * - logger is not a high-performance app
+		 */
+		if (write_all(ctl->fd, "\n", 1) < 0)
+			warn(_("write failed"));
 	if (ctl->stderr_printout)
 		fprintf(stderr, "%s\n", buf);
 }
@@ -387,29 +386,35 @@ static void syslog_rfc3164_header(struct logger_ctl *const ctl)
 #define NILVALUE "-"
 static void syslog_rfc5424_header(struct logger_ctl *const ctl)
 {
+	char *time;
+	char *hostname;
+	char *const app_name = ctl->tag;
+	char *procid;
+	char *const msgid = xstrdup(ctl->msgid ? ctl->msgid : NILVALUE);
+	char *structured_data;
+
 	if (ctl->fd < 0)
 		return;
 
-	char *time;
 	if (ctl->rfc5424_time) {
 		struct timeval tv;
 		struct tm *tm;
+
 		gettimeofday(&tv, NULL);
 		if ((tm = localtime(&tv.tv_sec)) != NULL) {
 			char fmt[64];
 			const size_t i = strftime(fmt, sizeof(fmt),
-					"%Y-%m-%dT%H:%M:%S.%%06u%z ", tm);
+						  "%Y-%m-%dT%H:%M:%S.%%06u%z ", tm);
 			/* patch TZ info to comply with RFC3339 (we left SP at end) */
-			fmt[i-1] = fmt[i-2];
-			fmt[i-2] = fmt[i-3];
-			fmt[i-3] = ':';
+			fmt[i - 1] = fmt[i - 2];
+			fmt[i - 2] = fmt[i - 3];
+			fmt[i - 3] = ':';
 			xasprintf(&time, fmt, tv.tv_usec);
 		} else
 			err(EXIT_FAILURE, _("localtime() failed"));
 	} else
 		time = xstrdup(NILVALUE);
 
-	char *hostname;
 	if (ctl->rfc5424_host) {
 		hostname = xgethostname();
 		/* Arbitrary looking 'if (var < strlen()) checks originate from
@@ -420,22 +425,18 @@ static void syslog_rfc5424_header(struct logger_ctl *const ctl)
 	} else
 		hostname = xstrdup(NILVALUE);
 
-	char *const app_name = ctl->tag;
 	if (48 < strlen(ctl->tag))
 		errx(EXIT_FAILURE, _("tag '%s' is too long"), ctl->tag);
 
-	char *procid;
 	if (ctl->pid)
 		xasprintf(&procid, "%d", ctl->pid);
 	else
 		procid = xstrdup(NILVALUE);
 
-	char *const msgid = xstrdup((ctl->msgid) ? ctl->msgid : NILVALUE);
-
-	char *structured_data;
 	if (ctl->rfc5424_tq) {
 #ifdef HAVE_NTP_GETTIME
 		struct ntptimeval ntptv;
+
 		if (ntp_gettime(&ntptv) == TIME_OK)
 			xasprintf(&structured_data,
 				 "[timeQuality tzKnown=\"1\" isSynced=\"1\" syncAccuracy=\"%ld\"]",
@@ -527,8 +528,8 @@ static void logger_open(struct logger_ctl *ctl)
 	ctl->fd = unix_socket(ctl, ctl->unix_socket, ctl->socket_type);
 	if (!ctl->syslogfp)
 		ctl->syslogfp = syslog_local_header;
-	if(!ctl->tag)
-			ctl->tag = xgetlogin();
+	if (!ctl->tag)
+		ctl->tag = xgetlogin();
 	generate_syslog_header(ctl);
 }
 
@@ -581,9 +582,9 @@ static void logger_stdin(struct logger_ctl *ctl)
 			if (c == '<') {
 				pri = 0;
 				buf[i++] = c;
-				while(isdigit(c = getchar()) && pri <= 191) {
-						buf[i++] = c;
-						pri = pri * 10 + c - '0';
+				while (isdigit(c = getchar()) && pri <= 191) {
+					buf[i++] = c;
+					pri = pri * 10 + c - '0';
 				}
 				if (c != EOF && c != '\n')
 					buf[i++] = c;
@@ -611,10 +612,10 @@ static void logger_stdin(struct logger_ctl *ctl)
 		}
 		buf[i] = '\0';
 
-		if(i > 0 || !ctl->skip_empty_lines)
+		if (i > 0 || !ctl->skip_empty_lines)
 			write_output(ctl, buf);
 
-		if (c == '\n') /* discard line terminator */
+		if (c == '\n')	/* discard line terminator */
 			c = getchar();
 	}
 }
@@ -701,29 +702,29 @@ int main(int argc, char **argv)
 	FILE *jfd = NULL;
 #endif
 	static const struct option longopts[] = {
-		{ "id",		optional_argument,  0, OPT_ID },
-		{ "stderr",	no_argument,	    0, 's' },
-		{ "file",	required_argument,  0, 'f' },
-		{ "priority",	required_argument,  0, 'p' },
-		{ "tag",	required_argument,  0, 't' },
-		{ "socket",	required_argument,  0, 'u' },
+		{ "id",		   optional_argument, 0, OPT_ID		   },
+		{ "stderr",	   no_argument,	      0, 's'		   },
+		{ "file",	   required_argument, 0, 'f'		   },
+		{ "priority",	   required_argument, 0, 'p'		   },
+		{ "tag",	   required_argument, 0, 't'		   },
+		{ "socket",	   required_argument, 0, 'u'		   },
 		{ "socket-errors", required_argument, 0, OPT_SOCKET_ERRORS },
-		{ "udp",	no_argument,	    0, 'd' },
-		{ "tcp",	no_argument,	    0, 'T' },
-		{ "server",	required_argument,  0, 'n' },
-		{ "port",	required_argument,  0, 'P' },
-		{ "version",	no_argument,	    0, 'V' },
-		{ "help",	no_argument,	    0, 'h' },
-		{ "prio-prefix", no_argument, 0, OPT_PRIO_PREFIX },
-		{ "rfc3164",	no_argument,  0, OPT_RFC3164 },
-		{ "rfc5424",	optional_argument,  0, OPT_RFC5424 },
-		{ "size",       required_argument,  0, 'S' },
-		{ "msgid",      required_argument,  0, OPT_MSGID },
-		{ "skip-empty", no_argument,  0, 'e' },
+		{ "udp",	   no_argument,	      0, 'd'		   },
+		{ "tcp",	   no_argument,	      0, 'T'		   },
+		{ "server",	   required_argument, 0, 'n'		   },
+		{ "port",	   required_argument, 0, 'P'		   },
+		{ "version",	   no_argument,	      0, 'V'		   },
+		{ "help",	   no_argument,	      0, 'h'		   },
+		{ "prio-prefix",   no_argument,	      0, OPT_PRIO_PREFIX   },
+		{ "rfc3164",	   no_argument,	      0, OPT_RFC3164	   },
+		{ "rfc5424",	   optional_argument, 0, OPT_RFC5424	   },
+		{ "size",	   required_argument, 0, 'S'		   },
+		{ "msgid",	   required_argument, 0, OPT_MSGID	   },
+		{ "skip-empty",	   no_argument,	      0, 'e'		   },
 #ifdef HAVE_LIBSYSTEMD
-		{ "journald",   optional_argument,  0, OPT_JOURNALD },
+		{ "journald",	   optional_argument, 0, OPT_JOURNALD	   },
 #endif
-		{ NULL,		0, 0, 0 }
+		{ NULL,		   0,		      0, 0		   }
 	};
 
 	setlocale(LC_ALL, "");
@@ -800,8 +801,8 @@ int main(int argc, char **argv)
 				parse_rfc5424_flags(&ctl, optarg);
 			break;
 		case OPT_MSGID:
-			if(strchr(optarg, ' '))
-					err(EXIT_FAILURE, _("--msgid cannot contain space"));
+			if (strchr(optarg, ' '))
+				err(EXIT_FAILURE, _("--msgid cannot contain space"));
 			ctl.msgid = optarg;
 			break;
 #ifdef HAVE_LIBSYSTEMD
-- 
2.3.2

Re: [PATCH 1/8] logger: tidy few indentation issues

[Karel Zak]

On Sun, Mar 15, 2015 at 12:54:47PM +0000, Sami Kerola wrote:
>  misc-utils/logger.c | 115 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 58 insertions(+), 57 deletions(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 1/8] logger: tidy few indentation issues

[Rainer Gerhards]

On Sun, Mar 15, 2015 at 1:54 PM, Sami Kerola <kerolasa@iki.fi> wrote:

Quick question on this (especially for future work):

> @@ -387,29 +386,35 @@ static void syslog_rfc3164_header(struct logger_ctl *const ctl)
>  #define NILVALUE "-"
>  static void syslog_rfc5424_header(struct logger_ctl *const ctl)
>  {
> +       char *time;
> +       char *hostname;
> +       char *const app_name = ctl->tag;
> +       char *procid;
> +       char *const msgid = xstrdup(ctl->msgid ? ctl->msgid : NILVALUE);
> +       char *structured_data;

Is it project policy to always define variables at the beginning of
the function? I ask because I used to do that as well, before I got
educated last year that it helps the compiler to produce better code
if variables are declared where needed.

Also on the idention: I assume indention via tabs with tabwidth 4. Is
there a style document I an look at?

Thanks and sorry for the extra effort.

Rainer

Re: [PATCH 1/8] logger: tidy few indentation issues

[Karel Zak]

On Mon, Mar 16, 2015 at 12:22:51PM +0100, Rainer Gerhards wrote:
> On Sun, Mar 15, 2015 at 1:54 PM, Sami Kerola <kerolasa@iki.fi> wrote:
> 
> Quick question on this (especially for future work):
> 
> > @@ -387,29 +386,35 @@ static void syslog_rfc3164_header(struct logger_ctl *const ctl)
> >  #define NILVALUE "-"
> >  static void syslog_rfc5424_header(struct logger_ctl *const ctl)
> >  {
> > +       char *time;
> > +       char *hostname;
> > +       char *const app_name = ctl->tag;
> > +       char *procid;
> > +       char *const msgid = xstrdup(ctl->msgid ? ctl->msgid : NILVALUE);
> > +       char *structured_data;
> 
> Is it project policy to always define variables at the beginning of
> the function? I ask because I used to do that as well, before I got

 at beginning of the { block }, for example:

 if (foo) {
    char *foostr = strdup(whatever);

    do_foo(foostr);
    free(foostr);
 }

 is fine.

> Also on the idention: I assume indention via tabs with tabwidth 4. Is
> there a style document I an look at?

 If possible (e.g. new/refactored stuff) then we usually follow kernel:
 https://www.kernel.org/doc/Documentation/CodingStyle

 ... but there are many old utils where it's better to follow already
 used codding style than mix more styles together.

> Thanks and sorry for the extra effort.

 Ah, this is a real detail...

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

[PATCH 2/8] logger: check xgethostname() return value

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/logger.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/misc-utils/logger.c b/misc-utils/logger.c
index e3fc248..2e37d14 100644
--- a/misc-utils/logger.c
+++ b/misc-utils/logger.c
@@ -342,9 +342,10 @@ static void write_output(const struct logger_ctl *ctl, const char *const msg)
 		fprintf(stderr, "%s\n", buf);
 }
 
+#define NILVALUE "-"
 static void syslog_rfc3164_header(struct logger_ctl *const ctl)
 {
-	char pid[30], *hostname, *dot;
+	char pid[30], *hostname;
 
 	*pid = '\0';
 	if (ctl->fd < 0)
@@ -352,10 +353,12 @@ static void syslog_rfc3164_header(struct logger_ctl *const ctl)
 	if (ctl->pid)
 		snprintf(pid, sizeof(pid), "[%d]", ctl->pid);
 
-	hostname = xgethostname();
-	dot = strchr(hostname, '.');
-	if (dot)
-		*dot = '\0';
+	if ((hostname = xgethostname())) {
+		char *dot = strchr(hostname, '.');
+		if (dot)
+			*dot = '\0';
+	} else
+		hostname = xstrdup(NILVALUE);
 
 	xasprintf(&ctl->hdr, "<%d>%.15s %s %.200s%s: ",
 		 ctl->pri, rfc3164_current_time(), hostname, ctl->tag, pid);
@@ -383,7 +386,6 @@ static void syslog_rfc3164_header(struct logger_ctl *const ctl)
  * specified RFC5424. The rest of the field mappings should be
  * pretty clear from RFC5424. -- Rainer Gerhards, 2015-03-10
  */
-#define NILVALUE "-"
 static void syslog_rfc5424_header(struct logger_ctl *const ctl)
 {
 	char *time;
@@ -416,7 +418,8 @@ static void syslog_rfc5424_header(struct logger_ctl *const ctl)
 		time = xstrdup(NILVALUE);
 
 	if (ctl->rfc5424_host) {
-		hostname = xgethostname();
+		if (!(hostname = xgethostname()))
+			hostname = xstrdup(NILVALUE);
 		/* Arbitrary looking 'if (var < strlen()) checks originate from
 		 * RFC 5424 - 6 Syslog Message Format definition.  */
 		if (255 < strlen(hostname))
-- 
2.3.2

Re: [PATCH 2/8] logger: check xgethostname() return value

[Karel Zak]

On Sun, Mar 15, 2015 at 12:54:48PM +0000, Sami Kerola wrote:
>  misc-utils/logger.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)

 Applied, thanks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

[PATCH 3/8] tests: add test_logger helper command

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/Makemodule.am |  4 ++++
 misc-utils/logger.c      | 36 ++++++++++++++++++++++++++++++++++++
 tests/commands.sh        |  1 +
 3 files changed, 41 insertions(+)

diff --git a/misc-utils/Makemodule.am b/misc-utils/Makemodule.am
index f7485c9..309727b 100644
--- a/misc-utils/Makemodule.am
+++ b/misc-utils/Makemodule.am
@@ -26,6 +26,10 @@ if HAVE_SYSTEMD
 logger_LDADD = $(SYSTEMD_LIBS) $(SYSTEMD_DAEMON_LIBS) $(SYSTEMD_JOURNAL_LIBS)
 logger_CFLAGS = $(SYSTEMD_CFLAGS) $(SYSTEMD_DAEMON_CFLAGS) $(SYSTEMD_JOURNAL_CFLAGS)
 endif
+check_PROGRAMS += test_logger
+test_logger_SOURCES = $(logger_SOURCES)
+test_logger_LDADD = $(logger_LDADD)
+test_logger_CFLAGS = -DTEST_LOGGER $(logger_CFLAGS)
 endif # BUILD_LOGGER
 
 
diff --git a/misc-utils/logger.c b/misc-utils/logger.c
index 2e37d14..838d225 100644
--- a/misc-utils/logger.c
+++ b/misc-utils/logger.c
@@ -310,7 +310,12 @@ static const char *rfc3164_current_time(void)
 		"Sep", "Oct", "Nov", "Dec"
 	};
 
+#ifndef TEST_LOGGER
 	gettimeofday(&tv, NULL);
+#else
+	tv.tv_sec = 1234567890;
+	tv.tv_usec = 123456;
+#endif
 	tm = localtime(&tv.tv_sec);
 	snprintf(time, sizeof(time),"%s %2d %2.2d:%2.2d:%2.2d",
 		monthnames[tm->tm_mon], tm->tm_mday,
@@ -326,7 +331,9 @@ static const char *rfc3164_current_time(void)
 static void write_output(const struct logger_ctl *ctl, const char *const msg)
 {
 	char *buf;
+#ifndef TEST_LOGGER
 	const size_t len = xasprintf(&buf, "%s%s", ctl->hdr, msg);
+
 	if (write_all(ctl->fd, buf, len) < 0)
 		warn(_("write failed"));
 	else if (ctl->socket_type == TYPE_TCP)
@@ -338,6 +345,9 @@ static void write_output(const struct logger_ctl *ctl, const char *const msg)
 		 */
 		if (write_all(ctl->fd, "\n", 1) < 0)
 			warn(_("write failed"));
+#else
+	xasprintf(&buf, "%s%s", ctl->hdr, msg);
+#endif
 	if (ctl->stderr_printout)
 		fprintf(stderr, "%s\n", buf);
 }
@@ -353,12 +363,16 @@ static void syslog_rfc3164_header(struct logger_ctl *const ctl)
 	if (ctl->pid)
 		snprintf(pid, sizeof(pid), "[%d]", ctl->pid);
 
+#ifndef TEST_LOGGER
 	if ((hostname = xgethostname())) {
 		char *dot = strchr(hostname, '.');
 		if (dot)
 			*dot = '\0';
 	} else
 		hostname = xstrdup(NILVALUE);
+#else
+	hostname = xstrdup("test-hostname");
+#endif
 
 	xasprintf(&ctl->hdr, "<%d>%.15s %s %.200s%s: ",
 		 ctl->pri, rfc3164_current_time(), hostname, ctl->tag, pid);
@@ -402,7 +416,12 @@ static void syslog_rfc5424_header(struct logger_ctl *const ctl)
 		struct timeval tv;
 		struct tm *tm;
 
+#ifndef TEST_LOGGER
 		gettimeofday(&tv, NULL);
+#else
+		tv.tv_sec = 1234567890;
+		tv.tv_usec = 123456;
+#endif
 		if ((tm = localtime(&tv.tv_sec)) != NULL) {
 			char fmt[64];
 			const size_t i = strftime(fmt, sizeof(fmt),
@@ -418,8 +437,12 @@ static void syslog_rfc5424_header(struct logger_ctl *const ctl)
 		time = xstrdup(NILVALUE);
 
 	if (ctl->rfc5424_host) {
+#ifndef TEST_LOGGER
 		if (!(hostname = xgethostname()))
 			hostname = xstrdup(NILVALUE);
+#else
+		hostname = xstrdup("test-hostname");
+#endif
 		/* Arbitrary looking 'if (var < strlen()) checks originate from
 		 * RFC 5424 - 6 Syslog Message Format definition.  */
 		if (255 < strlen(hostname))
@@ -443,7 +466,12 @@ static void syslog_rfc5424_header(struct logger_ctl *const ctl)
 		if (ntp_gettime(&ntptv) == TIME_OK)
 			xasprintf(&structured_data,
 				 "[timeQuality tzKnown=\"1\" isSynced=\"1\" syncAccuracy=\"%ld\"]",
+#ifndef TEST_LOGGER
 				 ntptv.maxerror);
+#else
+				 123456L);
+#endif
+
 		else
 #endif
 			xasprintf(&structured_data,
@@ -747,7 +775,11 @@ int main(int argc, char **argv)
 			ctl.skip_empty_lines = 1;
 			break;
 		case 'i':		/* log process id also */
+#ifndef TEST_LOGGER
 			ctl.pid = getpid();
+#else
+			ctl.pid = 98765;
+#endif
 			break;
 		case OPT_ID:
 			if (optarg) {
@@ -757,7 +789,11 @@ int main(int argc, char **argv)
 					p++;
 				ctl.pid = strtoul_or_err(optarg, _("failed to parse id"));
 			} else
+#ifndef TEST_LOGGER
 				ctl.pid = getpid();
+#else
+				ctl.pid = 98765;
+#endif
 			break;
 		case 'p':		/* priority */
 			ctl.pri = pencode(optarg);
diff --git a/tests/commands.sh b/tests/commands.sh
index e11bd2a..fd07b17 100644
--- a/tests/commands.sh
+++ b/tests/commands.sh
@@ -17,6 +17,7 @@ TS_HELPER_LIBMOUNT_DEBUG="$top_builddir/test_mount_debug"
 TS_HELPER_PYLIBMOUNT_CONTEXT="$top_srcdir/libmount/python/test_mount_context.py"
 TS_HELPER_PYLIBMOUNT_TAB="$top_srcdir/libmount/python/test_mount_tab.py"
 TS_HELPER_PYLIBMOUNT_UPDATE="$top_srcdir/libmount/python/test_mount_tab_update.py"
+TS_HELPER_LOGGER="$top_builddir/test_logger"
 TS_HELPER_LOGINDEFS="$top_builddir/test_logindefs"
 TS_HELPER_MD5="$top_builddir/test_md5"
 TS_HELPER_MORE=${TS_HELPER_MORE-"$top_builddir/test_more"}
-- 
2.3.2

Re: [PATCH 3/8] tests: add test_logger helper command

[Karel Zak]

On Sun, Mar 15, 2015 at 12:54:49PM +0000, Sami Kerola wrote:
> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  misc-utils/Makemodule.am |  4 ++++
>  misc-utils/logger.c      | 36 ++++++++++++++++++++++++++++++++++++
>  tests/commands.sh        |  1 +
>  3 files changed, 41 insertions(+)
> 
> diff --git a/misc-utils/Makemodule.am b/misc-utils/Makemodule.am
> index f7485c9..309727b 100644
> --- a/misc-utils/Makemodule.am
> +++ b/misc-utils/Makemodule.am
> @@ -26,6 +26,10 @@ if HAVE_SYSTEMD
>  logger_LDADD = $(SYSTEMD_LIBS) $(SYSTEMD_DAEMON_LIBS) $(SYSTEMD_JOURNAL_LIBS)
>  logger_CFLAGS = $(SYSTEMD_CFLAGS) $(SYSTEMD_DAEMON_CFLAGS) $(SYSTEMD_JOURNAL_CFLAGS)
>  endif
> +check_PROGRAMS += test_logger
> +test_logger_SOURCES = $(logger_SOURCES)
> +test_logger_LDADD = $(logger_LDADD)
> +test_logger_CFLAGS = -DTEST_LOGGER $(logger_CFLAGS)
>  endif # BUILD_LOGGER
>  
>  
> diff --git a/misc-utils/logger.c b/misc-utils/logger.c
> index 2e37d14..838d225 100644
> --- a/misc-utils/logger.c
> +++ b/misc-utils/logger.c
> @@ -310,7 +310,12 @@ static const char *rfc3164_current_time(void)
>  		"Sep", "Oct", "Nov", "Dec"
>  	};
>  
> +#ifndef TEST_LOGGER
>  	gettimeofday(&tv, NULL);
> +#else
> +	tv.tv_sec = 1234567890;
> +	tv.tv_usec = 123456;
> +#endif

 Hmm... this (many #ifdefs and hardcoded constants) is really not
 elegant solution. I'll try to implement it in another way. Thanks for
 the tests.

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 3/8] tests: add test_logger helper command

[Karel Zak]

On Sun, Mar 15, 2015 at 12:54:49PM +0000, Sami Kerola wrote:
> +#ifndef TEST_LOGGER
>  	gettimeofday(&tv, NULL);
> +#else
> +	tv.tv_sec = 1234567890;
> +	tv.tv_usec = 123456;
> +#endif

I have replaced this with logger_gettimeofday() macro and read the
value for the tests from LOGGER_TEST_TIMEOFDAY env.variable (and so on
for hostname and pid), for example to be compatible with your tests:

export TZ="GMT"
export LOGGER_TEST_TIMEOFDAY="1234567890.123456"
export LOGGER_TEST_HOSTNAME="test-hostname"
export LOGGER_TEST_GETPID="98765"

I have applied your test without a change (except the environment):

./run.sh logger

-------------------- util-linux regression tests --------------------

                    For development purpose only.                    
                 Don't execute on production system!                 

       logger: options                        ...
                : simple                      ... OK
                : log_pid                     ... OK
                : log_pid_long                ... OK
                : log_pid_define              ... OK
                : log_pid_no_arg              ... OK
                : input_file_simple           ... OK
                : input_file_empty_line       ... OK
                : input_file_skip_empty       ... OK
                : input_file_prio_prefix      ... OK
           ... OK (all 9 sub-tests PASSED)


It would be also nice to have tests for --rfc5424[=...] messages formatting.

   Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

[PATCH 4/8] tests: add logger(1) command line options tests

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 tests/expected/logger/input_empty_line             |  3 ++
 tests/expected/logger/input_prio_prefix            |  1 +
 tests/expected/logger/input_simple                 |  1 +
 tests/expected/logger/options                      |  0
 .../expected/logger/options-input_file_empty_line  |  3 ++
 .../expected/logger/options-input_file_prio_prefix |  1 +
 tests/expected/logger/options-input_file_simple    |  1 +
 .../expected/logger/options-input_file_skip_empty  |  2 +
 tests/expected/logger/options-log_pid              |  1 +
 tests/expected/logger/options-log_pid_define       |  1 +
 tests/expected/logger/options-log_pid_long         |  1 +
 tests/expected/logger/options-log_pid_no_arg       |  1 +
 tests/expected/logger/options-simple               |  1 +
 tests/ts/logger/options                            | 57 ++++++++++++++++++++++
 14 files changed, 74 insertions(+)
 create mode 100644 tests/expected/logger/input_empty_line
 create mode 100644 tests/expected/logger/input_prio_prefix
 create mode 100644 tests/expected/logger/input_simple
 create mode 100644 tests/expected/logger/options
 create mode 100644 tests/expected/logger/options-input_file_empty_line
 create mode 100644 tests/expected/logger/options-input_file_prio_prefix
 create mode 100644 tests/expected/logger/options-input_file_simple
 create mode 100644 tests/expected/logger/options-input_file_skip_empty
 create mode 100644 tests/expected/logger/options-log_pid
 create mode 100644 tests/expected/logger/options-log_pid_define
 create mode 100644 tests/expected/logger/options-log_pid_long
 create mode 100644 tests/expected/logger/options-log_pid_no_arg
 create mode 100644 tests/expected/logger/options-simple
 create mode 100755 tests/ts/logger/options

diff --git a/tests/expected/logger/input_empty_line b/tests/expected/logger/input_empty_line
new file mode 100644
index 0000000..78578ae
--- /dev/null
+++ b/tests/expected/logger/input_empty_line
@@ -0,0 +1,3 @@
+a1 a2 a3 a4 a5 b1 b2 b3 b4 b5 c1 c2 c3 c4 c5
+
+5{c..1} 4{c..1} 3{c..1} 2{c..1} 1{c..1}
diff --git a/tests/expected/logger/input_prio_prefix b/tests/expected/logger/input_prio_prefix
new file mode 100644
index 0000000..25a5f54
--- /dev/null
+++ b/tests/expected/logger/input_prio_prefix
@@ -0,0 +1 @@
+<66> prio_prefix
diff --git a/tests/expected/logger/input_simple b/tests/expected/logger/input_simple
new file mode 100644
index 0000000..9e5c66d
--- /dev/null
+++ b/tests/expected/logger/input_simple
@@ -0,0 +1 @@
+a1 a2 a3 a4 a5 b1 b2 b3 b4 b5 c1 c2 c3 c4 c5
diff --git a/tests/expected/logger/options b/tests/expected/logger/options
new file mode 100644
index 0000000..e69de29
diff --git a/tests/expected/logger/options-input_file_empty_line b/tests/expected/logger/options-input_file_empty_line
new file mode 100644
index 0000000..a10fd1c
--- /dev/null
+++ b/tests/expected/logger/options-input_file_empty_line
@@ -0,0 +1,3 @@
+<13>Feb 13 23:31:30 test_tag: a1 a2 a3 a4 a5 b1 b2 b3 b4 b5 c1 c2 c3 c4 c5
+<13>Feb 13 23:31:30 test_tag: 
+<13>Feb 13 23:31:30 test_tag: 5{c..1} 4{c..1} 3{c..1} 2{c..1} 1{c..1}
diff --git a/tests/expected/logger/options-input_file_prio_prefix b/tests/expected/logger/options-input_file_prio_prefix
new file mode 100644
index 0000000..b0f9785
--- /dev/null
+++ b/tests/expected/logger/options-input_file_prio_prefix
@@ -0,0 +1 @@
+<66>Feb 13 23:31:30 test_tag:  prio_prefix
diff --git a/tests/expected/logger/options-input_file_simple b/tests/expected/logger/options-input_file_simple
new file mode 100644
index 0000000..e8a2ca8
--- /dev/null
+++ b/tests/expected/logger/options-input_file_simple
@@ -0,0 +1 @@
+<13>Feb 13 23:31:30 test_tag: a1 a2 a3 a4 a5 b1 b2 b3 b4 b5 c1 c2 c3 c4 c5
diff --git a/tests/expected/logger/options-input_file_skip_empty b/tests/expected/logger/options-input_file_skip_empty
new file mode 100644
index 0000000..cdbe87a
--- /dev/null
+++ b/tests/expected/logger/options-input_file_skip_empty
@@ -0,0 +1,2 @@
+<13>Feb 13 23:31:30 test_tag: a1 a2 a3 a4 a5 b1 b2 b3 b4 b5 c1 c2 c3 c4 c5
+<13>Feb 13 23:31:30 test_tag: 5{c..1} 4{c..1} 3{c..1} 2{c..1} 1{c..1}
diff --git a/tests/expected/logger/options-log_pid b/tests/expected/logger/options-log_pid
new file mode 100644
index 0000000..d21ecd4
--- /dev/null
+++ b/tests/expected/logger/options-log_pid
@@ -0,0 +1 @@
+<13>Feb 13 23:31:30 test_tag[98765]: test
diff --git a/tests/expected/logger/options-log_pid_define b/tests/expected/logger/options-log_pid_define
new file mode 100644
index 0000000..14024b1
--- /dev/null
+++ b/tests/expected/logger/options-log_pid_define
@@ -0,0 +1 @@
+<13>Feb 13 23:31:30 test_tag[12345]: test
diff --git a/tests/expected/logger/options-log_pid_long b/tests/expected/logger/options-log_pid_long
new file mode 100644
index 0000000..d21ecd4
--- /dev/null
+++ b/tests/expected/logger/options-log_pid_long
@@ -0,0 +1 @@
+<13>Feb 13 23:31:30 test_tag[98765]: test
diff --git a/tests/expected/logger/options-log_pid_no_arg b/tests/expected/logger/options-log_pid_no_arg
new file mode 100644
index 0000000..d21ecd4
--- /dev/null
+++ b/tests/expected/logger/options-log_pid_no_arg
@@ -0,0 +1 @@
+<13>Feb 13 23:31:30 test_tag[98765]: test
diff --git a/tests/expected/logger/options-simple b/tests/expected/logger/options-simple
new file mode 100644
index 0000000..1d0c3a0
--- /dev/null
+++ b/tests/expected/logger/options-simple
@@ -0,0 +1 @@
+<13>Feb 13 23:31:30 test_tag: test
diff --git a/tests/ts/logger/options b/tests/ts/logger/options
new file mode 100755
index 0000000..f68b33f
--- /dev/null
+++ b/tests/ts/logger/options
@@ -0,0 +1,57 @@
+#!/bin/bash
+
+#
+# Copyright (C) 2015 Sami Kerola <kerolasa@iki.fi>
+#
+# This file is part of util-linux.
+#
+# This file 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 file 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.
+#
+
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="options"
+
+. $TS_TOPDIR/functions.sh
+
+ts_init "$*"
+
+ts_check_test_command "$TS_HELPER_LOGGER"
+
+# Create --file option input files.
+echo {a..c}{1..5}	 > $TS_OUTDIR/input_simple
+echo {a..c}{1..5}	 > $TS_OUTDIR/input_empty_line
+echo ""			>> $TS_OUTDIR/input_empty_line
+echo {5..1}{c..1}	>> $TS_OUTDIR/input_empty_line
+echo "<66>" prio_prefix	 > $TS_OUTDIR/input_prio_prefix
+
+# bash 4 might not be available, use go-around hash
+tests_array=(
+	"simple:--stderr test"
+	"log_pid:--stderr -i test"
+	"log_pid_long:--stderr --id test"
+	"log_pid_define:--stderr --id=12345 test"
+	"log_pid_no_arg:-is test"
+	"input_file_simple:--stderr -f $TS_OUTDIR/input_simple"
+	"input_file_empty_line:--stderr -f $TS_OUTDIR/input_empty_line"
+	"input_file_skip_empty:--stderr --file $TS_OUTDIR/input_empty_line -e"
+	"input_file_prio_prefix:--stderr --file $TS_OUTDIR/input_prio_prefix --skip-empty --prio-prefix"
+)
+
+for i in "${tests_array[@]}"; do
+	name="${i%%:*}"
+	options="${i##*:}"
+
+	ts_init_subtest "$name"
+	$TS_HELPER_LOGGER -t "test_tag" $options > "$TS_OUTPUT" 2>&1
+	ts_finalize_subtest
+done
+
+ts_finalize
-- 
2.3.2

[PATCH 5/8] logger: fix rfc5424 format crash

[Sami Kerola]

$ logger --rfc5424=notq message
Segmentation fault (core dumped)

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/logger.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/misc-utils/logger.c b/misc-utils/logger.c
index 838d225..b007377 100644
--- a/misc-utils/logger.c
+++ b/misc-utils/logger.c
@@ -476,7 +476,8 @@ static void syslog_rfc5424_header(struct logger_ctl *const ctl)
 #endif
 			xasprintf(&structured_data,
 				 "[timeQuality tzKnown=\"1\" isSynced=\"0\"]");
-	}
+	} else
+		structured_data = xstrdup("");
 
 	xasprintf(&ctl->hdr, "<%d>1 %s %s %s %s %s %s ",
 		ctl->pri,
-- 
2.3.2

Re: [PATCH 5/8] logger: fix rfc5424 format crash

[Karel Zak]

On Sun, Mar 15, 2015 at 12:54:51PM +0000, Sami Kerola wrote:
> $ logger --rfc5424=notq message
> Segmentation fault (core dumped)

Not sure if Rainer is subscribed to the list, so please keep him in CC.

> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> ---
>  misc-utils/logger.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/misc-utils/logger.c b/misc-utils/logger.c
> index 838d225..b007377 100644
> --- a/misc-utils/logger.c
> +++ b/misc-utils/logger.c
> @@ -476,7 +476,8 @@ static void syslog_rfc5424_header(struct logger_ctl *const ctl)
>  #endif
>  			xasprintf(&structured_data,
>  				 "[timeQuality tzKnown=\"1\" isSynced=\"0\"]");
> -	}
> +	} else
> +		structured_data = xstrdup("");

RFC5424:

    STRUCTURED-DATA = NILVALUE / 1*SD-ELEMENT

    NILVALUE        = "-"


... so I guess "" is bad idea. I'll fix it before commit.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 5/8] logger: fix rfc5424 format crash

[Rainer Gerhards]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

2015-03-16 11:49 GMT+01:00 Karel Zak <kzak@redhat.com>:

> On Sun, Mar 15, 2015 at 12:54:51PM +0000, Sami Kerola wrote:
> > $ logger --rfc5424=notq message
> > Segmentation fault (core dumped)
>
> Not sure if Rainer is subscribed to the list, so please keep him in CC.
>

I am since a couple of day (but via a gmail alias for easier filtering).


>
> > Signed-off-by: Sami Kerola <kerolasa@iki.fi>
> > ---
> >  misc-utils/logger.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/misc-utils/logger.c b/misc-utils/logger.c
> > index 838d225..b007377 100644
> > --- a/misc-utils/logger.c
> > +++ b/misc-utils/logger.c
> > @@ -476,7 +476,8 @@ static void syslog_rfc5424_header(struct logger_ctl
> *const ctl)
> >  #endif
> >                       xasprintf(&structured_data,
> >                                "[timeQuality tzKnown=\"1\"
> isSynced=\"0\"]");
> > -     }
> > +     } else
> > +             structured_data = xstrdup("");
>
> RFC5424:
>
>     STRUCTURED-DATA = NILVALUE / 1*SD-ELEMENT
>
>     NILVALUE        = "-"
>
>
> ... so I guess "" is bad idea. I'll fix it before commit.
>

yeah, I overlooked that case. I should really find time to work on "make
check"...

The best thing to do is use

= xstrdup(NILVALUE)

I defined NILVALUE exactly for those cases (see rest of code).

Rainer

PS: I've been silent for a couple of days because my main projects demand
attention. Will soon post more again here.

>
>     Karel
>
> --
>  Karel Zak  <kzak@redhat.com>
>  http://karelzak.blogspot.com
>

[-- Attachment #2: Type: text/html, Size: 2738 bytes --]

Re: [PATCH 5/8] logger: fix rfc5424 format crash

[Karel Zak]

On Mon, Mar 16, 2015 at 11:55:36AM +0100, Rainer Gerhards wrote:
> The best thing to do is use
> 
> = xstrdup(NILVALUE)

Yeah,
https://github.com/karelzak/util-linux/commit/773df0fa2f6c12581cc8649a08b9d825456a247c

> 
> I defined NILVALUE exactly for those cases (see rest of code).
> 
> Rainer
> 
> PS: I've been silent for a couple of days because my main projects demand
> attention. Will soon post more again here.

No problem.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 5/8] logger: fix rfc5424 format crash

[Rainer Gerhards]

2015-03-16 11:49 GMT+01:00 Karel Zak <kzak@redhat.com>:
> On Sun, Mar 15, 2015 at 12:54:51PM +0000, Sami Kerola wrote:
>> $ logger --rfc5424=notq message
>> Segmentation fault (core dumped)
>
> Not sure if Rainer is subscribed to the list, so please keep him in CC.


I am since a couple of day (but via a gmail alias for easier filtering).

>
>> Signed-off-by: Sami Kerola <kerolasa@iki.fi>
>> ---
>>  misc-utils/logger.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/misc-utils/logger.c b/misc-utils/logger.c
>> index 838d225..b007377 100644
>> --- a/misc-utils/logger.c
>> +++ b/misc-utils/logger.c
>> @@ -476,7 +476,8 @@ static void syslog_rfc5424_header(struct logger_ctl *const ctl)
>>  #endif
>>                       xasprintf(&structured_data,
>>                                "[timeQuality tzKnown=\"1\" isSynced=\"0\"]");
>> -     }
>> +     } else
>> +             structured_data = xstrdup("");
>
> RFC5424:
>
>     STRUCTURED-DATA = NILVALUE / 1*SD-ELEMENT
>
>     NILVALUE        = "-"
>
>
> ... so I guess "" is bad idea. I'll fix it before commit.

yeah, I overlooked that case. I should really find time to work on
"make check"...

The best thing to do is use

= xstrdup(NILVALUE)

I defined NILVALUE exactly for those cases (see rest of code).

Rainer

PS: I've been silent for a couple of days because my main projects
demand attention. Will soon post more again here.

>
>     Karel
>
> --
>  Karel Zak  <kzak@redhat.com>
>  http://karelzak.blogspot.com

[PATCH 6/8] tests: add logger(1) message format tests

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 tests/expected/logger/formats                |   0
 tests/expected/logger/formats-priorities     | 152 +++++++++++++++++++++++++++
 tests/expected/logger/formats-rfc3164        |   1 +
 tests/expected/logger/formats-rfc5424_msgid  |   1 +
 tests/expected/logger/formats-rfc5424_nohost |   1 +
 tests/expected/logger/formats-rfc5424_notime |   1 +
 tests/expected/logger/formats-rfc5424_simple |   1 +
 tests/ts/logger/formats                      |  53 ++++++++++
 8 files changed, 210 insertions(+)
 create mode 100644 tests/expected/logger/formats
 create mode 100644 tests/expected/logger/formats-priorities
 create mode 100644 tests/expected/logger/formats-rfc3164
 create mode 100644 tests/expected/logger/formats-rfc5424_msgid
 create mode 100644 tests/expected/logger/formats-rfc5424_nohost
 create mode 100644 tests/expected/logger/formats-rfc5424_notime
 create mode 100644 tests/expected/logger/formats-rfc5424_simple
 create mode 100755 tests/ts/logger/formats

diff --git a/tests/expected/logger/formats b/tests/expected/logger/formats
new file mode 100644
index 0000000..e69de29
diff --git a/tests/expected/logger/formats-priorities b/tests/expected/logger/formats-priorities
new file mode 100644
index 0000000..32d136f
--- /dev/null
+++ b/tests/expected/logger/formats-priorities
@@ -0,0 +1,152 @@
+<32>Feb 13 23:31:30 prio: auth.emerg
+<33>Feb 13 23:31:30 prio: auth.alert
+<34>Feb 13 23:31:30 prio: auth.crit
+<35>Feb 13 23:31:30 prio: auth.err
+<36>Feb 13 23:31:30 prio: auth.warning
+<37>Feb 13 23:31:30 prio: auth.notice
+<38>Feb 13 23:31:30 prio: auth.info
+<39>Feb 13 23:31:30 prio: auth.debug
+<80>Feb 13 23:31:30 prio: authpriv.emerg
+<81>Feb 13 23:31:30 prio: authpriv.alert
+<82>Feb 13 23:31:30 prio: authpriv.crit
+<83>Feb 13 23:31:30 prio: authpriv.err
+<84>Feb 13 23:31:30 prio: authpriv.warning
+<85>Feb 13 23:31:30 prio: authpriv.notice
+<86>Feb 13 23:31:30 prio: authpriv.info
+<87>Feb 13 23:31:30 prio: authpriv.debug
+<72>Feb 13 23:31:30 prio: cron.emerg
+<73>Feb 13 23:31:30 prio: cron.alert
+<74>Feb 13 23:31:30 prio: cron.crit
+<75>Feb 13 23:31:30 prio: cron.err
+<76>Feb 13 23:31:30 prio: cron.warning
+<77>Feb 13 23:31:30 prio: cron.notice
+<78>Feb 13 23:31:30 prio: cron.info
+<79>Feb 13 23:31:30 prio: cron.debug
+<24>Feb 13 23:31:30 prio: daemon.emerg
+<25>Feb 13 23:31:30 prio: daemon.alert
+<26>Feb 13 23:31:30 prio: daemon.crit
+<27>Feb 13 23:31:30 prio: daemon.err
+<28>Feb 13 23:31:30 prio: daemon.warning
+<29>Feb 13 23:31:30 prio: daemon.notice
+<30>Feb 13 23:31:30 prio: daemon.info
+<31>Feb 13 23:31:30 prio: daemon.debug
+<88>Feb 13 23:31:30 prio: ftp.emerg
+<89>Feb 13 23:31:30 prio: ftp.alert
+<90>Feb 13 23:31:30 prio: ftp.crit
+<91>Feb 13 23:31:30 prio: ftp.err
+<92>Feb 13 23:31:30 prio: ftp.warning
+<93>Feb 13 23:31:30 prio: ftp.notice
+<94>Feb 13 23:31:30 prio: ftp.info
+<95>Feb 13 23:31:30 prio: ftp.debug
+<48>Feb 13 23:31:30 prio: lpr.emerg
+<49>Feb 13 23:31:30 prio: lpr.alert
+<50>Feb 13 23:31:30 prio: lpr.crit
+<51>Feb 13 23:31:30 prio: lpr.err
+<52>Feb 13 23:31:30 prio: lpr.warning
+<53>Feb 13 23:31:30 prio: lpr.notice
+<54>Feb 13 23:31:30 prio: lpr.info
+<55>Feb 13 23:31:30 prio: lpr.debug
+<16>Feb 13 23:31:30 prio: mail.emerg
+<17>Feb 13 23:31:30 prio: mail.alert
+<18>Feb 13 23:31:30 prio: mail.crit
+<19>Feb 13 23:31:30 prio: mail.err
+<20>Feb 13 23:31:30 prio: mail.warning
+<21>Feb 13 23:31:30 prio: mail.notice
+<22>Feb 13 23:31:30 prio: mail.info
+<23>Feb 13 23:31:30 prio: mail.debug
+<56>Feb 13 23:31:30 prio: news.emerg
+<57>Feb 13 23:31:30 prio: news.alert
+<58>Feb 13 23:31:30 prio: news.crit
+<59>Feb 13 23:31:30 prio: news.err
+<60>Feb 13 23:31:30 prio: news.warning
+<61>Feb 13 23:31:30 prio: news.notice
+<62>Feb 13 23:31:30 prio: news.info
+<63>Feb 13 23:31:30 prio: news.debug
+<40>Feb 13 23:31:30 prio: syslog.emerg
+<41>Feb 13 23:31:30 prio: syslog.alert
+<42>Feb 13 23:31:30 prio: syslog.crit
+<43>Feb 13 23:31:30 prio: syslog.err
+<44>Feb 13 23:31:30 prio: syslog.warning
+<45>Feb 13 23:31:30 prio: syslog.notice
+<46>Feb 13 23:31:30 prio: syslog.info
+<47>Feb 13 23:31:30 prio: syslog.debug
+<8>Feb 13 23:31:30 prio: user.emerg
+<9>Feb 13 23:31:30 prio: user.alert
+<10>Feb 13 23:31:30 prio: user.crit
+<11>Feb 13 23:31:30 prio: user.err
+<12>Feb 13 23:31:30 prio: user.warning
+<13>Feb 13 23:31:30 prio: user.notice
+<14>Feb 13 23:31:30 prio: user.info
+<15>Feb 13 23:31:30 prio: user.debug
+<64>Feb 13 23:31:30 prio: uucp.emerg
+<65>Feb 13 23:31:30 prio: uucp.alert
+<66>Feb 13 23:31:30 prio: uucp.crit
+<67>Feb 13 23:31:30 prio: uucp.err
+<68>Feb 13 23:31:30 prio: uucp.warning
+<69>Feb 13 23:31:30 prio: uucp.notice
+<70>Feb 13 23:31:30 prio: uucp.info
+<71>Feb 13 23:31:30 prio: uucp.debug
+<128>Feb 13 23:31:30 prio: local0.emerg
+<129>Feb 13 23:31:30 prio: local0.alert
+<130>Feb 13 23:31:30 prio: local0.crit
+<131>Feb 13 23:31:30 prio: local0.err
+<132>Feb 13 23:31:30 prio: local0.warning
+<133>Feb 13 23:31:30 prio: local0.notice
+<134>Feb 13 23:31:30 prio: local0.info
+<135>Feb 13 23:31:30 prio: local0.debug
+<136>Feb 13 23:31:30 prio: local1.emerg
+<137>Feb 13 23:31:30 prio: local1.alert
+<138>Feb 13 23:31:30 prio: local1.crit
+<139>Feb 13 23:31:30 prio: local1.err
+<140>Feb 13 23:31:30 prio: local1.warning
+<141>Feb 13 23:31:30 prio: local1.notice
+<142>Feb 13 23:31:30 prio: local1.info
+<143>Feb 13 23:31:30 prio: local1.debug
+<144>Feb 13 23:31:30 prio: local2.emerg
+<145>Feb 13 23:31:30 prio: local2.alert
+<146>Feb 13 23:31:30 prio: local2.crit
+<147>Feb 13 23:31:30 prio: local2.err
+<148>Feb 13 23:31:30 prio: local2.warning
+<149>Feb 13 23:31:30 prio: local2.notice
+<150>Feb 13 23:31:30 prio: local2.info
+<151>Feb 13 23:31:30 prio: local2.debug
+<152>Feb 13 23:31:30 prio: local3.emerg
+<153>Feb 13 23:31:30 prio: local3.alert
+<154>Feb 13 23:31:30 prio: local3.crit
+<155>Feb 13 23:31:30 prio: local3.err
+<156>Feb 13 23:31:30 prio: local3.warning
+<157>Feb 13 23:31:30 prio: local3.notice
+<158>Feb 13 23:31:30 prio: local3.info
+<159>Feb 13 23:31:30 prio: local3.debug
+<160>Feb 13 23:31:30 prio: local4.emerg
+<161>Feb 13 23:31:30 prio: local4.alert
+<162>Feb 13 23:31:30 prio: local4.crit
+<163>Feb 13 23:31:30 prio: local4.err
+<164>Feb 13 23:31:30 prio: local4.warning
+<165>Feb 13 23:31:30 prio: local4.notice
+<166>Feb 13 23:31:30 prio: local4.info
+<167>Feb 13 23:31:30 prio: local4.debug
+<168>Feb 13 23:31:30 prio: local5.emerg
+<169>Feb 13 23:31:30 prio: local5.alert
+<170>Feb 13 23:31:30 prio: local5.crit
+<171>Feb 13 23:31:30 prio: local5.err
+<172>Feb 13 23:31:30 prio: local5.warning
+<173>Feb 13 23:31:30 prio: local5.notice
+<174>Feb 13 23:31:30 prio: local5.info
+<175>Feb 13 23:31:30 prio: local5.debug
+<176>Feb 13 23:31:30 prio: local6.emerg
+<177>Feb 13 23:31:30 prio: local6.alert
+<178>Feb 13 23:31:30 prio: local6.crit
+<179>Feb 13 23:31:30 prio: local6.err
+<180>Feb 13 23:31:30 prio: local6.warning
+<181>Feb 13 23:31:30 prio: local6.notice
+<182>Feb 13 23:31:30 prio: local6.info
+<183>Feb 13 23:31:30 prio: local6.debug
+<184>Feb 13 23:31:30 prio: local7.emerg
+<185>Feb 13 23:31:30 prio: local7.alert
+<186>Feb 13 23:31:30 prio: local7.crit
+<187>Feb 13 23:31:30 prio: local7.err
+<188>Feb 13 23:31:30 prio: local7.warning
+<189>Feb 13 23:31:30 prio: local7.notice
+<190>Feb 13 23:31:30 prio: local7.info
+<191>Feb 13 23:31:30 prio: local7.debug
diff --git a/tests/expected/logger/formats-rfc3164 b/tests/expected/logger/formats-rfc3164
new file mode 100644
index 0000000..0353460
--- /dev/null
+++ b/tests/expected/logger/formats-rfc3164
@@ -0,0 +1 @@
+<13>Feb 13 23:31:30 test-hostname rfc3164: message
diff --git a/tests/expected/logger/formats-rfc5424_msgid b/tests/expected/logger/formats-rfc5424_msgid
new file mode 100644
index 0000000..2ebe0de
--- /dev/null
+++ b/tests/expected/logger/formats-rfc5424_msgid
@@ -0,0 +1 @@
+<13>1 2009-02-13T23:31:30.123456+00:00 test-hostname rfc5424 - MSGID [timeQuality tzKnown="1" isSynced="1" syncAccuracy="123456"] message
diff --git a/tests/expected/logger/formats-rfc5424_nohost b/tests/expected/logger/formats-rfc5424_nohost
new file mode 100644
index 0000000..15d80a2
--- /dev/null
+++ b/tests/expected/logger/formats-rfc5424_nohost
@@ -0,0 +1 @@
+<13>1 2009-02-13T23:31:30.123456+00:00 - rfc5424 - - [timeQuality tzKnown="1" isSynced="1" syncAccuracy="123456"] message
diff --git a/tests/expected/logger/formats-rfc5424_notime b/tests/expected/logger/formats-rfc5424_notime
new file mode 100644
index 0000000..1b434de
--- /dev/null
+++ b/tests/expected/logger/formats-rfc5424_notime
@@ -0,0 +1 @@
+<13>1 - test-hostname rfc5424 - -  message
diff --git a/tests/expected/logger/formats-rfc5424_simple b/tests/expected/logger/formats-rfc5424_simple
new file mode 100644
index 0000000..b14789e
--- /dev/null
+++ b/tests/expected/logger/formats-rfc5424_simple
@@ -0,0 +1 @@
+<13>1 2009-02-13T23:31:30.123456+00:00 test-hostname rfc5424 - - [timeQuality tzKnown="1" isSynced="1" syncAccuracy="123456"] message
diff --git a/tests/ts/logger/formats b/tests/ts/logger/formats
new file mode 100755
index 0000000..cb7f61b
--- /dev/null
+++ b/tests/ts/logger/formats
@@ -0,0 +1,53 @@
+#!/bin/bash
+
+#
+# Copyright (C) 2015 Sami Kerola <kerolasa@iki.fi>
+#
+# This file is part of util-linux.
+#
+# This file 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 file 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.
+#
+
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="formats"
+
+. $TS_TOPDIR/functions.sh
+
+ts_init "$*"
+
+ts_check_test_command "$TS_HELPER_LOGGER"
+
+ts_init_subtest "rfc3164"
+$TS_HELPER_LOGGER -s -t "rfc3164" --rfc3164 "message" > "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+
+ts_init_subtest "rfc5424_simple"
+$TS_HELPER_LOGGER -s -t "rfc5424" --rfc5424 "message" > "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+ts_init_subtest "rfc5424_notime"
+$TS_HELPER_LOGGER -s -t "rfc5424" --rfc5424=notime "message" > "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+ts_init_subtest "rfc5424_nohost"
+$TS_HELPER_LOGGER -s -t "rfc5424" --rfc5424=nohost "message" > "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+ts_init_subtest "rfc5424_msgid"
+$TS_HELPER_LOGGER -s -t "rfc5424" --rfc5424 --msgid "MSGID" "message" > "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+
+ts_init_subtest "priorities"
+for facility in auth authpriv cron daemon ftp lpr mail news syslog user uucp local{0..7}; do
+	for level in emerg alert crit err warning notice info debug; do
+		$TS_HELPER_LOGGER -s -t "prio" -p "$facility.$level" "$facility.$level" >> "$TS_OUTPUT" 2>&1
+	done
+done
+ts_finalize_subtest
+
+ts_finalize
-- 
2.3.2

[PATCH 7/8] logger: use errx() when checking user input

[Sami Kerola]

Additionally inform in usage() the --msgid requires an argument.

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 misc-utils/logger.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/misc-utils/logger.c b/misc-utils/logger.c
index b007377..45d427e 100644
--- a/misc-utils/logger.c
+++ b/misc-utils/logger.c
@@ -684,7 +684,7 @@ static void __attribute__ ((__noreturn__)) usage(FILE *out)
 	fputs(_("     --rfc3164            use the obsolete BSD syslog protocol\n"), out);
 	fputs(_("     --rfc5424[=<snip>]   use the syslog protocol (the default for remote);\n"
 		"                            <snip> can be notime, or notq, and/or nohost\n"), out);
-	fputs(_("     --msgid              set rfc5424 MSGID field, ignored for non-rfc5424 format\n"), out);
+	fputs(_("     --msgid <msgid>      set rfc5424 message id field\n"), out);
 	fputs(_(" -u, --socket <socket>    write to this Unix socket\n"), out);
 	fputs(_("     --socket-errors[=<on|off|auto>]\n"
 		"                          print connection errors when using Unix sockets\n"), out);
@@ -842,7 +842,7 @@ int main(int argc, char **argv)
 			break;
 		case OPT_MSGID:
 			if (strchr(optarg, ' '))
-				err(EXIT_FAILURE, _("--msgid cannot contain space"));
+				errx(EXIT_FAILURE, _("--msgid cannot contain space"));
 			ctl.msgid = optarg;
 			break;
 #ifdef HAVE_LIBSYSTEMD
-- 
2.3.2

Re: [PATCH 7/8] logger: use errx() when checking user input

[Karel Zak]

On Sun, Mar 15, 2015 at 12:54:53PM +0000, Sami Kerola wrote:
>  misc-utils/logger.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

 Applied, thanks.

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

[PATCH 8/8] tests: add logger(1) error condition tests

[Sami Kerola]

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 tests/expected/logger/errors                       |  0
 tests/expected/logger/errors-id_with_space         |  2 +
 tests/expected/logger/errors-invalid_prio          |  1 +
 tests/expected/logger/errors-kern_priority         |  1 +
 tests/expected/logger/errors-kern_priority_numeric |  1 +
 tests/expected/logger/errors-rfc5424_exceed_size   |  1 +
 .../logger/errors-rfc5424_msgid_with_space         |  1 +
 tests/expected/logger/errors-tag_with_space        |  2 +
 tests/ts/logger/errors                             | 60 ++++++++++++++++++++++
 9 files changed, 69 insertions(+)
 create mode 100644 tests/expected/logger/errors
 create mode 100644 tests/expected/logger/errors-id_with_space
 create mode 100644 tests/expected/logger/errors-invalid_prio
 create mode 100644 tests/expected/logger/errors-kern_priority
 create mode 100644 tests/expected/logger/errors-kern_priority_numeric
 create mode 100644 tests/expected/logger/errors-rfc5424_exceed_size
 create mode 100644 tests/expected/logger/errors-rfc5424_msgid_with_space
 create mode 100644 tests/expected/logger/errors-tag_with_space
 create mode 100755 tests/ts/logger/errors

diff --git a/tests/expected/logger/errors b/tests/expected/logger/errors
new file mode 100644
index 0000000..e69de29
diff --git a/tests/expected/logger/errors-id_with_space b/tests/expected/logger/errors-id_with_space
new file mode 100644
index 0000000..081efac
--- /dev/null
+++ b/tests/expected/logger/errors-id_with_space
@@ -0,0 +1,2 @@
+test_logger: failed to parse id: 'A B'
+test_logger: failed to parse id: 'A B'
diff --git a/tests/expected/logger/errors-invalid_prio b/tests/expected/logger/errors-invalid_prio
new file mode 100644
index 0000000..cf79cf3
--- /dev/null
+++ b/tests/expected/logger/errors-invalid_prio
@@ -0,0 +1 @@
+test_logger: unknown priority name: 8
diff --git a/tests/expected/logger/errors-kern_priority b/tests/expected/logger/errors-kern_priority
new file mode 100644
index 0000000..6656db0
--- /dev/null
+++ b/tests/expected/logger/errors-kern_priority
@@ -0,0 +1 @@
+<8>Feb 13 23:31:30 prio: message
diff --git a/tests/expected/logger/errors-kern_priority_numeric b/tests/expected/logger/errors-kern_priority_numeric
new file mode 100644
index 0000000..6656db0
--- /dev/null
+++ b/tests/expected/logger/errors-kern_priority_numeric
@@ -0,0 +1 @@
+<8>Feb 13 23:31:30 prio: message
diff --git a/tests/expected/logger/errors-rfc5424_exceed_size b/tests/expected/logger/errors-rfc5424_exceed_size
new file mode 100644
index 0000000..35750b7
--- /dev/null
+++ b/tests/expected/logger/errors-rfc5424_exceed_size
@@ -0,0 +1 @@
+<13>1 2009-02-13T23:31:30.123456+00:00 test-hostname rfc5424_exceed_size - - [timeQuality tzKnown="1" isSynced="1" syncAccuracy="123456"] abc
diff --git a/tests/expected/logger/errors-rfc5424_msgid_with_space b/tests/expected/logger/errors-rfc5424_msgid_with_space
new file mode 100644
index 0000000..273bc9b
--- /dev/null
+++ b/tests/expected/logger/errors-rfc5424_msgid_with_space
@@ -0,0 +1 @@
+test_logger: --msgid cannot contain space
diff --git a/tests/expected/logger/errors-tag_with_space b/tests/expected/logger/errors-tag_with_space
new file mode 100644
index 0000000..31a86af
--- /dev/null
+++ b/tests/expected/logger/errors-tag_with_space
@@ -0,0 +1,2 @@
+<13>Feb 13 23:31:30 A B: tag_with_space
+<13>1 2009-02-13T23:31:30.123456+00:00 test-hostname A B - - [timeQuality tzKnown="1" isSynced="1" syncAccuracy="123456"] tag_with_space_rfc5424
diff --git a/tests/ts/logger/errors b/tests/ts/logger/errors
new file mode 100755
index 0000000..1051b55
--- /dev/null
+++ b/tests/ts/logger/errors
@@ -0,0 +1,60 @@
+#!/bin/bash
+
+#
+# Copyright (C) 2015 Sami Kerola <kerolasa@iki.fi>
+#
+# This file is part of util-linux.
+#
+# This file 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 file 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.
+#
+
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="errors"
+
+. $TS_TOPDIR/functions.sh
+
+ts_init "$*"
+
+ts_check_test_command "$TS_HELPER_LOGGER"
+
+ts_init_subtest "kern_priority"
+$TS_HELPER_LOGGER -s -t "prio" -p kern.emerg "message" > "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+
+ts_init_subtest "kern_priority_numeric"
+$TS_HELPER_LOGGER -s -t "prio" -p 0 "message" > "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+
+ts_init_subtest "invalid_prio"
+$TS_HELPER_LOGGER -s -t "prio" -p 8 "message" > "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+
+# should truncate
+ts_init_subtest "rfc5424_exceed_size"
+$TS_HELPER_LOGGER -s -t "rfc5424_exceed_size" --rfc5424 --size 3 "abcd" > "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+
+ts_init_subtest "id_with_space"
+$TS_HELPER_LOGGER -s -t "id_with_space" --id="A B" "message" > "$TS_OUTPUT" 2>&1
+$TS_HELPER_LOGGER -s -t "rfc5424_id_with_space" --rfc5424 --id="A B" "message" >> "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+
+# should not fail
+ts_init_subtest "tag_with_space"
+$TS_HELPER_LOGGER -s -t "A B" "tag_with_space" > "$TS_OUTPUT" 2>&1
+$TS_HELPER_LOGGER -s -t "A B" --rfc5424 "tag_with_space_rfc5424" >> "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+
+ts_init_subtest "rfc5424_msgid_with_space"
+$TS_HELPER_LOGGER -s -t "rfc5424_msgid_with_space" --rfc5424 --msgid="A B" "message" > "$TS_OUTPUT" 2>&1
+ts_finalize_subtest
+
+ts_finalize
-- 
2.3.2

Re: [PATCH 0/8] pull: logger: add tests and fix couple issues

[Rainer Gerhards]

On Sun, Mar 15, 2015 at 1:54 PM, Sami Kerola <kerolasa@iki.fi> wrote:
> Hello,
>
> Coverage of these tests is not magnificent.  None of the communications
> to external systems using tcp, udp, or to journald are tested, but at
> least some aspects of the logger(1) functionality gets to be checked -
> that is message formats and input validation.


ah! excellent. I'll also have a look to understand the nature of how
tests work for util-linux. Regarding tcp and udp: I guess a simple way
would be to use netcat? Or is that considered bad practice for
util-linux (as versions may differ and may interfere with test
results?

Thanks,
Rainer

Re: [PATCH 0/8] pull: logger: add tests and fix couple issues

[Karel Zak]

On Mon, Mar 16, 2015 at 12:28:19PM +0100, Rainer Gerhards wrote:
> On Sun, Mar 15, 2015 at 1:54 PM, Sami Kerola <kerolasa@iki.fi> wrote:
> > Hello,
> >
> > Coverage of these tests is not magnificent.  None of the communications
> > to external systems using tcp, udp, or to journald are tested, but at
> > least some aspects of the logger(1) functionality gets to be checked -
> > that is message formats and input validation.
> 
> 
> ah! excellent. I'll also have a look to understand the nature of how
> tests work for util-linux. Regarding tcp and udp: I guess a simple way
> would be to use netcat? Or is that considered bad practice for
> util-linux (as versions may differ and may interfere with test
> results?

 We have no any strict policy for tests, it's fine to check all
 necessary environment and versions and skip (ts_skip function) all
 the test if necessary. 
 
 We have two basic modes:
 
 * "non-root" (make check) - this is usually for simple tests where you
   don't need root permissions and where is it safe to call the test
   on arbitrary machine (to make it usable for example from spec
   files on build machines, etc.)

 * "root" - this is for dangerous tests you never want to call on
   production system, it's never executed by build system and you
   have to manually "cd tests; ./run.sh". This is for example for
   mount(8), some fdisk tests etc. These tests are for developers
   only.



 Note I have just committed a new --no-act logger command line option
 to make it possible to check messages formatting without spam in
 system logs, for example:

   $ logger --no-act --stderr --rfc5424 "This is message"

 I guess we can use it for messages formatting tests (as the tests
 suggested by Sami).

    Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 0/8] pull: logger: add tests and fix couple issues

[Sami Kerola]

On Mon, 16 Mar 2015, Karel Zak wrote:

> On Mon, Mar 16, 2015 at 12:28:19PM +0100, Rainer Gerhards wrote:
> > On Sun, Mar 15, 2015 at 1:54 PM, Sami Kerola <kerolasa@iki.fi> wrote:
> > > Hello,
> > >
> > > Coverage of these tests is not magnificent.  None of the communications
> > > to external systems using tcp, udp, or to journald are tested, but at
> > > least some aspects of the logger(1) functionality gets to be checked -
> > > that is message formats and input validation.
> > 
> > 
> > ah! excellent. I'll also have a look to understand the nature of how
> > tests work for util-linux. Regarding tcp and udp: I guess a simple way
> > would be to use netcat? Or is that considered bad practice for
> > util-linux (as versions may differ and may interfere with test
> > results?
> 
>  We have no any strict policy for tests, it's fine to check all
>  necessary environment and versions and skip (ts_skip function) all
>  the test if necessary. 
>  
>  We have two basic modes:
>  
>  * "non-root" (make check) - this is usually for simple tests where you
>    don't need root permissions and where is it safe to call the test
>    on arbitrary machine (to make it usable for example from spec
>    files on build machines, etc.)
> 
>  * "root" - this is for dangerous tests you never want to call on
>    production system, it's never executed by build system and you
>    have to manually "cd tests; ./run.sh". This is for example for
>    mount(8), some fdisk tests etc. These tests are for developers
>    only.
> 
> 
> 
>  Note I have just committed a new --no-act logger command line option
>  to make it possible to check messages formatting without spam in
>  system logs, for example:
> 
>    $ logger --no-act --stderr --rfc5424 "This is message"
> 
>  I guess we can use it for messages formatting tests (as the tests
>  suggested by Sami).

Good midnight,

Since there is now --no-act, and the test_logger reads environment 
variables what time stamps etc to use the two commits sent earlier
with titles

tests: add logger(1) error condition tests
tests: add logger(1) message format tests

are updated in my remote repo.

https://github.com/kerolasa/lelux-utiliteetit/commit/b3d4c9862c1c12c1f1e5db5e72fe010b192ca106
https://github.com/kerolasa/lelux-utiliteetit/commit/660d2e44e5d75d98cfeb8971c7229f2c4b1d03d9

I also noticed journald messages can be sent with --no-act so I added new 
check on top of the two above. Here's the mail list version of it.

--->8----
From: Sami Kerola <kerolasa@iki.fi>
Date: Mon, 16 Mar 2015 23:42:57 +0000
Subject: [PATCH 3/4] tests: add logger(1) journald test

Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 tests/expected/logger/journald |  3 +++
 tests/ts/logger/journald       | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+)
 create mode 100644 tests/expected/logger/journald
 create mode 100755 tests/ts/logger/journald

diff --git a/tests/expected/logger/journald b/tests/expected/logger/journald
new file mode 100644
index 0000000..d4b1549
--- /dev/null
+++ b/tests/expected/logger/journald
@@ -0,0 +1,3 @@
+MESSAGE_ID=b8f74e14bc714bfc8040a5106dc9376a
+MESSAGE=a b c 1 2 3
+
diff --git a/tests/ts/logger/journald b/tests/ts/logger/journald
new file mode 100755
index 0000000..155aeae
--- /dev/null
+++ b/tests/ts/logger/journald
@@ -0,0 +1,35 @@
+#!/bin/bash
+
+#
+# Copyright (C) 2015 Sami Kerola <kerolasa@iki.fi>
+#
+# This file is part of util-linux.
+#
+# This file 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 file 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.
+#
+
+TS_TOPDIR="${0%/*}/../.."
+TS_DESC="journald"
+
+. $TS_TOPDIR/functions.sh
+
+ts_init "$*"
+
+ts_check_test_command "$TS_HELPER_LOGGER"
+
+if ! $TS_HELPER_LOGGER --help | grep -q journald; then
+	TS_KNOWN_FAIL="yes"
+fi
+
+printf "%s\n%s\n%s\n" MESSAGE_ID=b8f74e14bc714bfc8040a5106dc9376a MESSAGE="a b c 1 2 3" |
+$TS_HELPER_LOGGER --no-act --journald --stderr > "$TS_OUTPUT" 2>&1
+
+ts_finalize
-- 
2.3.3

Re: [PATCH 0/8] pull: logger: add tests and fix couple issues

[Karel Zak]

On Mon, Mar 16, 2015 at 11:55:05PM +0000, Sami Kerola wrote:
> are updated in my remote repo.
> 
> https://github.com/kerolasa/lelux-utiliteetit/commit/b3d4c9862c1c12c1f1e5db5e72fe010b192ca106
> https://github.com/kerolasa/lelux-utiliteetit/commit/660d2e44e5d75d98cfeb8971c7229f2c4b1d03d9

Merged, thanks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com