pty: Refactor how we capture output

This will now avoid copying all the data around between the kernel and
userland and will also simply use our own callbacks which helps us to
keep the code a little bit less of an if/else hell.

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

diff --git a/src/pakfire/jail.c b/src/pakfire/jail.c
index 55898878dc60335a4183366b5ae371e30bb4674f..d1d992dd901e4c6d32293015c7d5a9cbc176e40f 100644 (file)
--- a/src/pakfire/jail.c
+++ b/src/pakfire/jail.c
@@ -1409,10 +1409,6 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail,
        } else if (stdout_callback) {
                // Nothing
 
-       // Capture Output?
-       } else if (output) {
-               pty_flags |= PAKFIRE_PTY_CAPTURE_OUTPUT;
-
        // Otherwise we dump everything to the console
        } else {
                // XXX Need to find a solution about what to do here...
@@ -1433,6 +1429,15 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail,
        if (r)
                goto ERROR;
 
+       // Capture Output?
+       if (output) {
+               r = pakfire_pty_capture_output(ctx.pty, output, output_length);
+               if (r < 0) {
+                       ERROR(jail->ctx, "Failed to set up output capture: %s\n", strerror(-r));
+                       goto ERROR;
+               }
+       }
+
        // Configure the callbacks
        if (stdin_callback)
                pakfire_pty_set_stdin_callback(ctx.pty, stdin_callback, stdin_data);
@@ -1541,10 +1546,6 @@ static int __pakfire_jail_exec(struct pakfire_jail* jail,
                goto ERROR;
        }
 
-       // Return the output
-       if (output)
-               *output = pakfire_pty_output(ctx.pty, output_length);
-
        // Return the exit code
        r = ctx.exit;
 

diff --git a/src/pakfire/pty.c b/src/pakfire/pty.c
index ed5c56348af7b51aa0189bc83a4cca35c65dba0c..edbb100d7faa33347829a4bf110d23bc40067fe5 100644 (file)
--- a/src/pakfire/pty.c
+++ b/src/pakfire/pty.c
@@ -109,48 +109,14 @@ struct pakfire_pty {
                PAKFIRE_PTY_STATE_DONE,
        } state;
 
-       // Captured Output
-       struct iovec output;
+       // Stream for output
+       FILE* output;
 };
 
 static int pakfire_pty_has_flag(struct pakfire_pty* pty, int flag) {
        return pty->flags & flag;
 }
 
-static int pakfire_pty_store_output(struct pakfire_pty* pty) {
-       struct stat buffer;
-       int r;
-
-       // Check if we have a file descriptor
-       if (pty->stdout.fd < 0)
-               return -EBADF;
-
-       // Stat the buffer
-       r = fstat(pty->stdout.fd, &buffer);
-       if (r < 0) {
-               ERROR(pty->ctx, "Could not stat the output buffer: %m\n");
-               return -errno;
-       }
-
-       DEBUG(pty->ctx, "Read %jd byte(s) of output\n", buffer.st_size);
-
-       // Don't try to map an empty buffer
-       if (!buffer.st_size)
-               return 0;
-
-       // Map the output into memory
-       pty->output.iov_base = mmap(NULL, buffer.st_size, PROT_READ, MAP_SHARED, pty->stdout.fd, 0);
-       if (pty->output.iov_base == MAP_FAILED) {
-               ERROR(pty->ctx, "Could not map the output into memory: %m\n");
-               return -errno;
-       }
-
-       // Store the size of the buffer
-       pty->output.iov_len = buffer.st_size;
-
-       return 0;
-}
-
 static int pakfire_pty_same_inode(struct pakfire_pty* pty, int fd1, int fd2) {
        struct stat stat1;
        struct stat stat2;
@@ -318,19 +284,14 @@ static int pakfire_pty_drained(struct pakfire_pty* pty) {
 }
 
 static int pakfire_pty_done(struct pakfire_pty* pty, int code) {
-       int r;
-
        // Don't run this more than once
        if (pty->state == PAKFIRE_PTY_STATE_DONE)
                return 0;
 
-       // Store the output
-       if (pakfire_pty_has_flag(pty, PAKFIRE_PTY_CAPTURE_OUTPUT)) {
-               r = pakfire_pty_store_output(pty);
-               if (r < 0) {
-                       ERROR(pty->ctx, "Could not store output: %s\n", strerror(-r));
-                       return r;
-               }
+       // Close the output
+       if (pty->output) {
+               fclose(pty->output);
+               pty->output = NULL;
        }
 
        // Disconnect
@@ -1305,14 +1266,12 @@ static void pakfire_pty_free(struct pakfire_pty* pty) {
        if (pty->socket[1] >= 0)
                close(pty->socket[1]);
 
-       // Output
-       if (pty->output.iov_base)
-               munmap(pty->output.iov_base, pty->output.iov_len);
-
        if (pty->loop)
                sd_event_unref(pty->loop);
        if (pty->ctx)
                pakfire_ctx_unref(pty->ctx);
+       if (pty->output)
+               fclose(pty->output);
        free(pty);
 }
 
@@ -1557,34 +1516,29 @@ int pakfire_pty_open(struct pakfire_pty* pty) {
        return 0;
 }
 
-/*
-       Creates an independent copy of the output buffer
-*/
-char* pakfire_pty_output(struct pakfire_pty* pty, size_t* length) {
-       char* buffer = NULL;
-
-       // Is not operation supported?
-       if (!pakfire_pty_has_flag(pty, PAKFIRE_PTY_CAPTURE_OUTPUT)) {
-               errno = -ENOTSUP;
-               return NULL;
-       }
+static int __pakfire_pty_capture_output(struct pakfire_ctx* ctx,
+               void* data, const char* line, const size_t length) {
+       struct pakfire_pty* self = data;
 
-       if (!pty->output.iov_base)
-               return NULL;
+       // Write to the buffer
+       return fwrite(line, 1, length, self->output);
+}
 
-       // Allocate a new buffer
-       buffer = calloc(pty->output.iov_len + 1, sizeof(*buffer));
-       if (!buffer)
-               return NULL;
+int pakfire_pty_capture_output(struct pakfire_pty* self, char** output, size_t* length) {
+       if (!output || !length)
+               return -EINVAL;
 
-       // Copy the output
-       memcpy(buffer, pty->output.iov_base, pty->output.iov_len);
+       // Open the memory stream
+       self->output = open_memstream(output, length);
+       if (!self->output) {
+               ERROR(self->ctx, "Failed to open memory stream: %m\n");
+               return -errno;
+       }
 
-       // Return the length
-       if (length)
-               *length = pty->output.iov_len;
+       // Set the callback
+       pakfire_pty_set_stdout_callback(self, __pakfire_pty_capture_output, self);
 
-       return buffer;
+       return 0;
 }
 
 /*

diff --git a/src/pakfire/pty.h b/src/pakfire/pty.h
index 3627e1f1ef6ad02a839cd27cb9b4502ee90584e6..c4af39b48b16597079d3e30d44224b07865cecf5 100644 (file)
--- a/src/pakfire/pty.h
+++ b/src/pakfire/pty.h
@@ -43,7 +43,7 @@ struct pakfire_pty* pakfire_pty_unref(struct pakfire_pty* pty);
 
 int pakfire_pty_open(struct pakfire_pty* pty);
 
-char* pakfire_pty_output(struct pakfire_pty* pty, size_t* length);
+int pakfire_pty_capture_output(struct pakfire_pty* pty, char** output, size_t* length);
 
 typedef ssize_t (*pakfire_pty_stdin_callback)(
        struct pakfire_ctx* ctx, void* data, char* buffer, size_t length);

diff --git a/tests/libpakfire/jail.c b/tests/libpakfire/jail.c
index 1d22abb932277d4092c7221e8dfc7e384ea4e370..8f541032ef93d5cf770e83fbab8485332d3d765c 100644 (file)
--- a/tests/libpakfire/jail.c
+++ b/tests/libpakfire/jail.c
@@ -166,6 +166,7 @@ FAIL:
 static int test_nice(const struct test* t) {
        struct pakfire_jail* jail = NULL;
        char* output = NULL;
+       size_t length = 0;
        int r = EXIT_FAILURE;
 
        const char* argv[] = {
@@ -183,7 +184,7 @@ static int test_nice(const struct test* t) {
        ASSERT_SUCCESS(pakfire_jail_nice(jail, 5));
 
        // Check if the nice level has been set
-       ASSERT_SUCCESS(pakfire_jail_exec_capture_output(jail, argv, NULL, 0, &output, NULL));
+       ASSERT_SUCCESS(pakfire_jail_exec_capture_output(jail, argv, NULL, 0, &output, &length));
        ASSERT_STRING_EQUALS(output, "5\n");
 
        // Success