libpakfire: execute: Statically allocate the log buffer

This prevents the buffer from growing unboundedly, but limits us to only
process log messages of up to the buffer size.

I have chosen 64k which should be more than enought that we never run
into this situation.

Signed-off-by: Michael Tremer <michael.tremer@ipfire.org>

diff --git a/src/libpakfire/execute.c b/src/libpakfire/execute.c
index 02fd2f0a7acd52dd092d041a690cef7a126581fd..1c48b764d6cb2967c474eb219c5dd530dbfb3590 100644 (file)
--- a/src/libpakfire/execute.c
+++ b/src/libpakfire/execute.c
@@ -36,6 +36,7 @@
 #include <pakfire/private.h>
 #include <pakfire/types.h>
 
+#define BUFFER_SIZE      1024 * 64
 #define EPOLL_MAX_EVENTS 2
 
 static char* envp_empty[1] = { NULL };
@@ -51,46 +52,12 @@ struct pakfire_execute {
 };
 
 struct pakfire_execute_buffer {
-       char* data;
-       size_t size;
+       char data[BUFFER_SIZE];
        size_t used;
 };
 
-static int pakfire_execute_buffer_realloc(Pakfire pakfire, struct pakfire_execute_buffer* buffer, size_t size) {
-       // We cannot decrease the buffer size
-       if (size <= buffer->size)
-               return -EINVAL;
-
-       // Allocate the new buffer
-       buffer->data = realloc(buffer->data, size);
-       if (!buffer->data)
-               return -errno;
-
-       buffer->size = size;
-
-       DEBUG(pakfire, "Buffer %p is now %zu byte(s) long\n", buffer, buffer->size);
-
-       return 0;
-}
-
-static int pakfire_execute_buffer_init(Pakfire pakfire, struct pakfire_execute_buffer* buffer) {
-       // Initialize all values with nothing
-       buffer->data = NULL;
-       buffer->size = buffer->used = 0;
-
-       // Allocate one page
-       return pakfire_execute_buffer_realloc(pakfire, buffer, PAGE_SIZE);
-}
-
-static void pakfire_execute_buffer_free(struct pakfire_execute_buffer* buffer) {
-       if (buffer->data)
-               free(buffer->data);
-
-       buffer->size = buffer->used = 0;
-}
-
 static int pakfire_execute_buffer_is_full(const struct pakfire_execute_buffer* buffer) {
-       return (buffer->size == buffer->used);
+       return (sizeof(buffer->data) == buffer->used);
 }
 
 /*
@@ -100,10 +67,12 @@ static int pakfire_execute_buffer_is_full(const struct pakfire_execute_buffer* b
 */
 static int pakfire_execute_logger_proxy(Pakfire pakfire,
                int(*log_func)(Pakfire pakfire, const char* data), int fd, struct pakfire_execute_buffer* buffer) {
+       char line[BUFFER_SIZE + 1];
+
        // Fill up buffer from fd
-       while (buffer->used < buffer->size) {
+       while (buffer->used < sizeof(buffer->data)) {
                ssize_t bytes_read = read(fd, buffer->data + buffer->used,
-                               buffer->size - buffer->used);
+                               sizeof(buffer->data) - buffer->used);
 
                // Handle errors
                if (bytes_read < 0) {
@@ -128,27 +97,20 @@ static int pakfire_execute_logger_proxy(Pakfire pakfire,
 
                // No newline found
                if (!eol) {
-                       // If the buffer is full, we double its size and try to read more
+                       // If the buffer is full, we send the content to the logger and try again
+                       // This should not happen in practise
                        if (pakfire_execute_buffer_is_full(buffer)) {
-                               int r = pakfire_execute_buffer_realloc(pakfire, buffer, buffer->size * 2);
-                               if (r)
-                                       return -1;
-
-                               return pakfire_execute_logger_proxy(pakfire, log_func, fd, buffer);
-                       }
+                               ERROR(pakfire, "Logging buffer is full. Sending all content\n");
+                               eol = buffer->data + sizeof(buffer->data) - 1;
 
                        // Otherwise we might have only read parts of the output
-                       return 0;
+                       } else
+                               break;
                }
 
                // Find the length of the string
                size_t length = eol - buffer->data + 1;
 
-               DEBUG(pakfire, "Found a line of %zu byte(s) length\n", length);
-
-               // Allocate a new buffer that is large enough to hold the line
-               char* line = alloca(length + 1);
-
                // Copy the line into the buffer
                memcpy(line, buffer->data, length);
 
@@ -184,18 +146,7 @@ static int pakfire_execute_logger(Pakfire pakfire, struct pakfire_execute_logger
                struct pakfire_execute_buffer stdout;
                struct pakfire_execute_buffer stderr;
        } buffers;
-
-       r = pakfire_execute_buffer_init(pakfire, &buffers.stdout);
-       if (r) {
-               ERROR(pakfire, "Could not initialize buffer for stdout: %s\n", strerror(errno));
-               goto OUT;
-       }
-
-       r = pakfire_execute_buffer_init(pakfire, &buffers.stderr);
-       if (r) {
-               ERROR(pakfire, "Could not initialize buffer for stderr: %s\n", strerror(errno));
-               goto OUT;
-       }
+       buffers.stdout.used = buffers.stderr.used = 0;
 
        // Setup epoll
        epollfd = epoll_create1(0);
@@ -272,10 +223,6 @@ OUT:
        if (epollfd > 0)
                close(epollfd);
 
-       // Free buffers
-       pakfire_execute_buffer_free(&buffers.stdout);
-       pakfire_execute_buffer_free(&buffers.stderr);
-
        return r;
 }
 

diff --git a/tests/python/execute.py b/tests/python/execute.py
index f8719411ca1703977139b24111c96f8ff465c9c6..25f60f112bc7ea8451a2fbaa7fd3091307fd0fd7 100755 (executable)
--- a/tests/python/execute.py
+++ b/tests/python/execute.py
@@ -61,8 +61,8 @@ class Test(unittest.TestCase):
        def test_execute_output(self):
                self.pakfire.execute(["/bin/bash", "--help"])
 
-               # Run a command with a lot of output which will increase the buffer sizes
-               self.pakfire.execute(["/usr/bin/openssl", "rand", "-hex", "4096"])
+               # Run a command with a lot of output which exceeds the buffer size
+               self.pakfire.execute(["/usr/bin/openssl", "rand", "-hex", "65536"])
 
                # Run a command that generates lots of lines
                self.pakfire.execute(["/usr/bin/openssl", "rand", "-base64", "4096"])