Re: [systemd-devel] [PATCH 2/3] journal: use audit event names instead of numbers

2015-04-23 Thread Zbigniew Jędrzejewski-Szmek 
On Mon, Apr 20, 2015 at 05:06:33PM +0200, Lennart Poettering wrote:
 On Mon, 20.04.15 14:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
 
  On Mon, Apr 20, 2015 at 04:43:20PM +0200, Lennart Poettering wrote:
   On Tue, 14.04.15 21:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) 
   wrote:
   
+const char *audit_type_name(int type,
+char buf[AUDIT_NAME_BUF_SIZE]) {
+const char *s;
+
+s = audit_type_to_string(type);
+if (s)
+return s;
+
+/* This is inspired by DNS TYPEnnn formatting */
+snprintf(buf, AUDIT_NAME_BUF_SIZE, AUDIT%04i, type);
+return buf;
   
   Shouldn't we stick to audit-xyz at least, to stay congruent to what
   we so far did, at least for the unknown ones?
  I thought the visual difference between the capitalized names and what we
  used so far is too big: e.g. ADD_USER and audit-1114, and it is better
  to have something visually similar at least.
 
 Hmm, I see. You do have a point, hence go ahead.
 
   Also, may turn this into a macro expression using ({}) that returns
   this as alloca() allocated string? 
 
  You mean the whole function? I'll try that. The only downside is it
  cannot be called directly in the args to another function.
 
 Yeah, the whole function.
 
 Make sure to name this in a way that it is clear that this is an
 alloca()-based macro, by including an _alloca() suffix or so?
 
 Also see procfs_file_alloca() or inspiration.

I now pushed something along those lines. Seems simple enough
not to warrant another posting to the mailing list.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/3] journal: use audit event names instead of numbers

2015-04-20 Thread Lennart Poettering 
On Tue, 14.04.15 21:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

 +const char *audit_type_name(int type,
 +char buf[AUDIT_NAME_BUF_SIZE]) {
 +const char *s;
 +
 +s = audit_type_to_string(type);
 +if (s)
 +return s;
 +
 +/* This is inspired by DNS TYPEnnn formatting */
 +snprintf(buf, AUDIT_NAME_BUF_SIZE, AUDIT%04i, type);
 +return buf;

Shouldn't we stick to audit-xyz at least, to stay congruent to what
we so far did, at least for the unknown ones?

Also, may turn this into a macro expression using ({}) that returns
this as alloca() allocated string? 


 +}
 diff --git src/journal/audit-type.h src/journal/audit-type.h
 index 9f37716cd6..a2c98cee80 100644
 --- src/journal/audit-type.h
 +++ src/journal/audit-type.h
 @@ -21,6 +21,11 @@
along with systemd; If not, see http://www.gnu.org/licenses/.
  ***/
  
 +#include macro.h
  
  const char *audit_type_to_string(int type);
  int audit_type_from_string(const char *s);
 +
 +#define AUDIT_NAME_BUF_SIZE sizeof(AUDIT)-1 +
  DECIMAL_STR_MAX(int)

Will break if people use expressions like 3*AUDIT_NAME_BUF_SIZE, since
it is missing the surrounding ().

 +const char *audit_type_name(int type,
 +char buf[AUDIT_NAME_BUF_SIZE]);

Weird line break...

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/3] journal: use audit event names instead of numbers

2015-04-20 Thread Zbigniew Jędrzejewski-Szmek 
On Mon, Apr 20, 2015 at 04:43:20PM +0200, Lennart Poettering wrote:
 On Tue, 14.04.15 21:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:
 
  +const char *audit_type_name(int type,
  +char buf[AUDIT_NAME_BUF_SIZE]) {
  +const char *s;
  +
  +s = audit_type_to_string(type);
  +if (s)
  +return s;
  +
  +/* This is inspired by DNS TYPEnnn formatting */
  +snprintf(buf, AUDIT_NAME_BUF_SIZE, AUDIT%04i, type);
  +return buf;
 
 Shouldn't we stick to audit-xyz at least, to stay congruent to what
 we so far did, at least for the unknown ones?
I thought the visual difference between the capitalized names and what we
used so far is too big: e.g. ADD_USER and audit-1114, and it is better
to have something visually similar at least.
 
 Also, may turn this into a macro expression using ({}) that returns
 this as alloca() allocated string? 
You mean the whole function? I'll try that. The only downside is it
cannot be called directly in the args to another function.

  +}
  diff --git src/journal/audit-type.h src/journal/audit-type.h
  index 9f37716cd6..a2c98cee80 100644
  --- src/journal/audit-type.h
  +++ src/journal/audit-type.h
  @@ -21,6 +21,11 @@
 along with systemd; If not, see http://www.gnu.org/licenses/.
   ***/
   
  +#include macro.h
   
   const char *audit_type_to_string(int type);
   int audit_type_from_string(const char *s);
  +
  +#define AUDIT_NAME_BUF_SIZE sizeof(AUDIT)-1 +
   DECIMAL_STR_MAX(int)
 
 Will break if people use expressions like 3*AUDIT_NAME_BUF_SIZE, since
 it is missing the surrounding ().
OK.

  +const char *audit_type_name(int type,
  +char buf[AUDIT_NAME_BUF_SIZE]);
OK.

Thanks for the review, I'll submit another version.

Zbyszek
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

Re: [systemd-devel] [PATCH 2/3] journal: use audit event names instead of numbers

2015-04-20 Thread Lennart Poettering 
On Mon, 20.04.15 14:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) wrote:

 On Mon, Apr 20, 2015 at 04:43:20PM +0200, Lennart Poettering wrote:
  On Tue, 14.04.15 21:58, Zbigniew Jędrzejewski-Szmek (zbys...@in.waw.pl) 
  wrote:
  
   +const char *audit_type_name(int type,
   +char buf[AUDIT_NAME_BUF_SIZE]) {
   +const char *s;
   +
   +s = audit_type_to_string(type);
   +if (s)
   +return s;
   +
   +/* This is inspired by DNS TYPEnnn formatting */
   +snprintf(buf, AUDIT_NAME_BUF_SIZE, AUDIT%04i, type);
   +return buf;
  
  Shouldn't we stick to audit-xyz at least, to stay congruent to what
  we so far did, at least for the unknown ones?
 I thought the visual difference between the capitalized names and what we
 used so far is too big: e.g. ADD_USER and audit-1114, and it is better
 to have something visually similar at least.

Hmm, I see. You do have a point, hence go ahead.

  Also, may turn this into a macro expression using ({}) that returns
  this as alloca() allocated string? 

 You mean the whole function? I'll try that. The only downside is it
 cannot be called directly in the args to another function.

Yeah, the whole function.

Make sure to name this in a way that it is clear that this is an
alloca()-based macro, by including an _alloca() suffix or so?

Also see procfs_file_alloca() or inspiration.

Lennart

-- 
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH 2/3] journal: use audit event names instead of numbers

2015-04-14 Thread Zbigniew Jędrzejewski-Szmek 
audit-1400 is replaced by AVC, etc.

A fallback mechanism is provided for unlisted event types.
Occasionally new types are added to the kernel, but not too often.

Add a simple test, which simply prints the mapping.
---
 .gitignore|  1 +
 Makefile.am   |  9 -
 src/journal/audit-type.c  | 14 ++
 src/journal/audit-type.h  |  5 +
 src/journal/journald-audit.c  | 11 +++
 src/journal/test-audit-type.c | 40 
 6 files changed, 75 insertions(+), 5 deletions(-)
 create mode 100644 src/journal/test-audit-type.c

diff --git .gitignore .gitignore
index bcf21feddd..9da5122339 100644
--- .gitignore
+++ .gitignore
@@ -141,6 +141,7 @@
 /systemd-vconsole-setup
 /tags
 /test-architecture
+/test-audit-type
 /test-async
 /test-barrier
 /test-boot-timestamp
diff --git Makefile.am Makefile.am
index 4c8812bba0..2d82fd12c1 100644
--- Makefile.am
+++ Makefile.am
@@ -4527,6 +4527,12 @@ test_compress_benchmark_LDADD = \
libsystemd-journal-internal.la \
libsystemd-shared.la
 
+test_audit_type_SOURCES = \
+   src/journal/test-audit-type.c
+
+test_audit_type_LDADD = \
+   libsystemd-journal-core.la
+
 libsystemd_journal_core_la_SOURCES = \
src/journal/journald-kmsg.c \
src/journal/journald-kmsg.h \
@@ -4609,7 +4615,8 @@ tests += \
test-journal-interleaving \
test-journal-flush \
test-mmap-cache \
-   test-catalog
+   test-catalog \
+   test-audit-type
 
 if HAVE_COMPRESSION
 tests += \
diff --git src/journal/audit-type.c src/journal/audit-type.c
index 9e6d4cb76b..03830c94f1 100644
--- src/journal/audit-type.c
+++ src/journal/audit-type.c
@@ -19,6 +19,7 @@
   along with systemd; If not, see http://www.gnu.org/licenses/.
 ***/
 
+#include stdio.h
 #include linux/audit.h
 
 #include audit-type.h
@@ -40,3 +41,16 @@ int audit_type_from_string(const char *s) {
 
 return sc-id;
 }
+
+const char *audit_type_name(int type,
+char buf[AUDIT_NAME_BUF_SIZE]) {
+const char *s;
+
+s = audit_type_to_string(type);
+if (s)
+return s;
+
+/* This is inspired by DNS TYPEnnn formatting */
+snprintf(buf, AUDIT_NAME_BUF_SIZE, AUDIT%04i, type);
+return buf;
+}
diff --git src/journal/audit-type.h src/journal/audit-type.h
index 9f37716cd6..a2c98cee80 100644
--- src/journal/audit-type.h
+++ src/journal/audit-type.h
@@ -21,6 +21,11 @@
   along with systemd; If not, see http://www.gnu.org/licenses/.
 ***/
 
+#include macro.h
 
 const char *audit_type_to_string(int type);
 int audit_type_from_string(const char *s);
+
+#define AUDIT_NAME_BUF_SIZE sizeof(AUDIT)-1 + DECIMAL_STR_MAX(int)
+const char *audit_type_name(int type,
+char buf[AUDIT_NAME_BUF_SIZE]);
diff --git src/journal/journald-audit.c src/journal/journald-audit.c
index 46eb82fa34..6677670161 100644
--- src/journal/journald-audit.c
+++ src/journal/journald-audit.c
@@ -21,6 +21,7 @@
 
 #include missing.h
 #include journald-audit.h
+#include audit-type.h
 
 typedef struct MapField {
 const char *audit_field;
@@ -336,11 +337,12 @@ static void process_audit_string(Server *s, int type, 
const char *data, size_t s
 size_t n_iov_allocated = 0;
 unsigned n_iov = 0, k;
 uint64_t seconds, msec, id;
-const char *p;
+const char *p, *type_name;
 unsigned z;
 char id_field[sizeof(_AUDIT_ID=) + DECIMAL_STR_MAX(uint64_t)],
  type_field[sizeof(_AUDIT_TYPE=) + DECIMAL_STR_MAX(int)],
- source_time_field[sizeof(_SOURCE_REALTIME_TIMESTAMP=) + 
DECIMAL_STR_MAX(usec_t)];
+ source_time_field[sizeof(_SOURCE_REALTIME_TIMESTAMP=) + 
DECIMAL_STR_MAX(usec_t)],
+ type_name_buf[AUDIT_NAME_BUF_SIZE];
 char *m;
 
 assert(s);
@@ -396,8 +398,9 @@ static void process_audit_string(Server *s, int type, const 
char *data, size_t s
 IOVEC_SET_STRING(iov[n_iov++], SYSLOG_FACILITY=32);
 IOVEC_SET_STRING(iov[n_iov++], SYSLOG_IDENTIFIER=audit);
 
-m = alloca(strlen(MESSAGE=audit-) + DECIMAL_STR_MAX(int) + 
strlen( ) + strlen(p) + 1);
-sprintf(m, MESSAGE=audit-%i %s, type, p);
+type_name = audit_type_name(type, type_name_buf);
+
+m = strjoina(MESSAGE=, type_name,  , p);
 IOVEC_SET_STRING(iov[n_iov++], m);
 
 z = n_iov;
diff --git src/journal/test-audit-type.c src/journal/test-audit-type.c
new file mode 100644
index 00..f8fe5b02c5
--- /dev/null
+++ src/journal/test-audit-type.c
@@ -0,0 +1,40 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+  This file is part of systemd.
+
+  Copyright 2015 Zbigniew Jędrzejewski-Szmek
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of