public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH mini-journalreader v2 1/2] fix #6410: properly close json output on error
@ 2025-06-02 14:30 Dominik Csapak
  2025-06-02 14:30 ` [pve-devel] [RFC PATCH mini-journalreader v2 2/2] include error in json output Dominik Csapak
  2025-06-03  8:53 ` [pve-devel] [PATCH mini-journalreader v2 1/2] fix #6410: properly close json output on error Dominik Csapak
  0 siblings, 2 replies; 4+ messages in thread
From: Dominik Csapak @ 2025-06-02 14:30 UTC (permalink / raw)
  To: pve-devel

when encountering an error (e.g. there was no curser at the wanted
position), the program would close with exitcode 1, but would not
properly finish the json output if in json mode.

To fix that, introduce a `print_error_and_exit` helper that tries to
finish the json output. We set `success` to 0 to indicate an error.

Ideally we would insert the error message into the json output, but
since that can be anything, and json expects utf8 and properly escaped
strings we'd either have to do a lot of things manually here, or use a
json library for escaping. So for now simply don't write the error in
the output.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from v1:
* use va_start/end and vfprintf, otherwise the variadic args won't work

 src/mini-journalreader.c | 34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/src/mini-journalreader.c b/src/mini-journalreader.c
index 98bcaac..562b390 100644
--- a/src/mini-journalreader.c
+++ b/src/mini-journalreader.c
@@ -30,6 +30,23 @@ static char BUF[BUFSIZE];
 bool json = false;
 bool first_line = true;
 
+// helper to print errors on stderr
+// if we're in json mode, print closing json body if possible
+static void print_error_and_exit(const char *fmt, ...) {
+    va_list args;
+    va_start(args, fmt);
+    vfprintf(stderr, fmt, args);
+    va_end(args);
+    if (json) {
+        size_t r = fwrite_unlocked("],\"success\":0}", 1, 14, stdout);
+        if (r < 14) {
+            fprintf(stderr, "Failed to write closing json\n");
+        }
+        fflush_unlocked(stdout);
+    }
+    exit(1);
+}
+
 static uint64_t get_timestamp(sd_journal *j) {
     uint64_t timestamp;
     int r = sd_journal_get_realtime_usec(j, &timestamp);
@@ -47,6 +64,7 @@ static void print_to_buf(const char * string, size_t length) {
 
     size_t r = fwrite_unlocked(string, 1, length, stdout);
     if (r < length) {
+        // don't use error helper here as we can't write to stdout anyway
         fprintf(stderr, "Failed to write\n");
         exit(1);
     }
@@ -57,8 +75,7 @@ static void print_cursor(sd_journal *j) {
     char *cursor = NULL;
     r = sd_journal_get_cursor(j, &cursor);
     if (r < 0) {
-        fprintf(stderr, "Failed to get cursor: %s\n", strerror(-r));
-        exit(1);
+        print_error_and_exit("Failed to get cursor: %s\n", strerror(-r));
     }
     if (json) {
         if (!first_line) {
@@ -242,6 +259,7 @@ static uint64_t arg_to_uint64(const char *argument) {
     uint64_t value = strtoull(argument, &end, 10);
     if (errno != 0 || *end != '\0') {
         fprintf(stderr, "%s is not a valid integer number\n", argument);
+        // don't use error helper here, as we did not start outputting json yet
         exit(1);
     }
 
@@ -348,15 +366,13 @@ int main(int argc, char *argv[]) {
         }
 
         if (r < 0) {
-            fprintf(stderr, "Failed to seek to end/cursor: %s\n", strerror(-r));
-            exit(1);
+            print_error_and_exit( "Failed to seek to end/cursor: %s\n", strerror(-r));
         }
 
         // seek back number entries and print cursor
         r = sd_journal_previous_skip(j, number + 1);
         if (r < 0) {
-            fprintf(stderr, "Failed to seek back: %s\n", strerror(-r));
-            exit(1);
+            print_error_and_exit("Failed to seek back: %s\n", strerror(-r));
         }
     } else {
         if (begin) {
@@ -368,16 +384,14 @@ int main(int argc, char *argv[]) {
         }
 
         if (r < 0) {
-            fprintf(stderr, "Failed to seek to begin/cursor: %s\n", strerror(-r));
-            exit(1);
+            print_error_and_exit("Failed to seek to begin/cursor: %s\n", strerror(-r));
         }
 
         // if we have a start cursor, we want to skip the first entry
         if (startcursor) {
             r = sd_journal_next(j);
             if (r < 0) {
-                fprintf(stderr, "Failed to seek to begin/cursor: %s\n", strerror(-r));
-                exit(1);
+                print_error_and_exit("Failed to seek to begin/cursor: %s\n", strerror(-r));
             }
             print_first_cursor(j);
         }
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] [RFC PATCH mini-journalreader v2 2/2] include error in json output
  2025-06-02 14:30 [pve-devel] [PATCH mini-journalreader v2 1/2] fix #6410: properly close json output on error Dominik Csapak
@ 2025-06-02 14:30 ` Dominik Csapak
  2025-06-03  7:16   ` Dominik Csapak
  2025-06-03  8:53 ` [pve-devel] [PATCH mini-journalreader v2 1/2] fix #6410: properly close json output on error Dominik Csapak
  1 sibling, 1 reply; 4+ messages in thread
From: Dominik Csapak @ 2025-06-02 14:30 UTC (permalink / raw)
  To: pve-devel

by using json-c and printing the string into a variable via `vasprintf`.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
changes from RFC v1:
* rebase on last patch and also use vfprintf in fallback case

 debian/control           |  2 +-
 src/Makefile             |  2 +-
 src/mini-journalreader.c | 31 +++++++++++++++++++++++++------
 3 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/debian/control b/debian/control
index 8280f6d..d03772e 100644
--- a/debian/control
+++ b/debian/control
@@ -2,7 +2,7 @@ Source: proxmox-mini-journalreader
 Section: admin
 Priority: optional
 Maintainer: Proxmox Support Team <support@proxmox.com>
-Build-Depends: debhelper-compat (= 13), libsystemd-dev, pkg-config, scdoc,
+Build-Depends: debhelper-compat (= 13), libsystemd-dev, pkg-config, scdoc, libjson-c-dev
 Standards-Version: 4.6.2
 
 Package: proxmox-mini-journalreader
diff --git a/src/Makefile b/src/Makefile
index 449004f..adb2782 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -4,7 +4,7 @@ SOURCES=mini-journalreader.c
 BIN_DIR ?= $(DESTDIR)/usr/bin
 MAN1_DIR ?= $(DESTDIR)/usr/share/man/man1
 
-LIBS := libsystemd
+LIBS := libsystemd json-c
 CFLAGS += -Werror -Wall -Wextra -Wl,-z,relro -g -O2 --std=gnu11
 CFLAGS += -fstack-protector-strong -D_FORTIFY_SOURCE=2
 CFLAGS += $(shell pkg-config --cflags $(LIBS))
diff --git a/src/mini-journalreader.c b/src/mini-journalreader.c
index 562b390..e3982ab 100644
--- a/src/mini-journalreader.c
+++ b/src/mini-journalreader.c
@@ -15,6 +15,10 @@
  * If not, see <https://www.gnu.org/licenses/>.
  */
 
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
 #include <errno.h>
 #include <stdbool.h>
 #include <stdio.h>
@@ -23,6 +27,7 @@
 #include <systemd/sd-journal.h>
 #include <time.h>
 #include <unistd.h>
+#include <json.h>
 
 #define BUFSIZE 4096
 
@@ -33,17 +38,31 @@ bool first_line = true;
 // helper to print errors on stderr
 // if we're in json mode, print closing json body if possible
 static void print_error_and_exit(const char *fmt, ...) {
+    fflush_unlocked(stdout);
+
     va_list args;
+    char *message;
     va_start(args, fmt);
-    vfprintf(stderr, fmt, args);
+    int r = vasprintf(&message, fmt, args);
+    if (r < 0) {
+        // could not allocate the string, so write it directly and don't
+        // include the error in the json output
+        vfprintf(stderr, fmt, args);
+        if (json) {
+            fprintf(stdout, "],\"success\":0}");
+        }
+        exit(1);
+    }
+    fprintf(stderr, "%s", message);
     va_end(args);
+
     if (json) {
-        size_t r = fwrite_unlocked("],\"success\":0}", 1, 14, stdout);
-        if (r < 14) {
-            fprintf(stderr, "Failed to write closing json\n");
-        }
-        fflush_unlocked(stdout);
+        struct json_object *obj = json_object_new_string(message);
+        const char *json_message = json_object_to_json_string(obj);
+        fprintf(stdout, "], \"error\":%s,\"success\":0}", json_message);
+        json_object_put(obj);
     }
+    free(message);
     exit(1);
 }
 
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pve-devel] [RFC PATCH mini-journalreader v2 2/2] include error in json output
  2025-06-02 14:30 ` [pve-devel] [RFC PATCH mini-journalreader v2 2/2] include error in json output Dominik Csapak
@ 2025-06-03  7:16   ` Dominik Csapak
  0 siblings, 0 replies; 4+ messages in thread
From: Dominik Csapak @ 2025-06-03  7:16 UTC (permalink / raw)
  To: pve-devel

on second though, we only ever print unknown strings from strerror here, which should be ascii only?
so using our already existing escaping of quotes and control characters should be enough...

will send a v3 that combines these things and drops the dependency on json-c...




_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pve-devel] [PATCH mini-journalreader v2 1/2] fix #6410: properly close json output on error
  2025-06-02 14:30 [pve-devel] [PATCH mini-journalreader v2 1/2] fix #6410: properly close json output on error Dominik Csapak
  2025-06-02 14:30 ` [pve-devel] [RFC PATCH mini-journalreader v2 2/2] include error in json output Dominik Csapak
@ 2025-06-03  8:53 ` Dominik Csapak
  1 sibling, 0 replies; 4+ messages in thread
From: Dominik Csapak @ 2025-06-03  8:53 UTC (permalink / raw)
  To: pve-devel

superseeded by v3:
https://lore.proxmox.com/pve-devel/20250603085247.917673-1-d.csapak@proxmox.com/T/#u


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-06-03  8:53 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-06-02 14:30 [pve-devel] [PATCH mini-journalreader v2 1/2] fix #6410: properly close json output on error Dominik Csapak
2025-06-02 14:30 ` [pve-devel] [RFC PATCH mini-journalreader v2 2/2] include error in json output Dominik Csapak
2025-06-03  7:16   ` Dominik Csapak
2025-06-03  8:53 ` [pve-devel] [PATCH mini-journalreader v2 1/2] fix #6410: properly close json output on error Dominik Csapak

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal