---
isys/imount.c | 83 +++++++++++++++++++++++++++++++++++---------------------
1 files changed, 52 insertions(+), 31 deletions(-)

diff --git a/isys/imount.c b/isys/imount.c
index a8e346e..516d309 100644
--- a/isys/imount.c
+++ b/isys/imount.c
@@ -29,6 +29,7 @@
#include <unistd.h>

#include "imount.h"
+#include "log.h"

#define _(foo) foo

@@ -68,11 +69,20 @@ static int readFD(int fd, char **buf) {
return filesize;
}

+static size_t rstrip(char *str) {
+ size_t len = strlen(str);
+ if (len > 0 && str[len-1] == '
') {
+ str[len-1] = '';
+ --len;
+ }
+ return len;
+}
+
int mountCommandWrapper(int mode, char *dev, char *where, char *fs,
char *options, char **err) {
- int rc, child, status, pipefd[2];
+ int rc, child, status;
+ int stdout_pipe[2], stderr_pipe[2];
char *opts = NULL, *device = NULL, *cmd = NULL;
- int programLogFD;

if (mode == IMOUNT_MODE_MOUNT) {
cmd = "/bin/mount";
@@ -118,69 +128,80 @@ int mountCommandWrapper(int mode, char *dev, char *where, char *fs,
}
}

- programLogFD = open("/tmp/program.log",
- O_APPEND|O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR|S_IRGRP|S_IROTH);
-
- if (pipe(pipefd))
+ if (pipe(stdout_pipe))
+ return IMOUNT_ERR_ERRNO;
+ if (pipe(stderr_pipe))
return IMOUNT_ERR_ERRNO;

if (!(child = fork())) {
- int fd;
+ int tty_fd;

- close(pipefd[0]);
+ close(stdout_pipe[0]);
+ close(stderr_pipe[0]);

- /* Close stdin entirely, redirect stdout to /tmp/program.log, and
- * redirect stderr to a pipe so we can put error messages into
- * exceptions. We'll only use these messages should mount also
- * return an error code.
+ /* Pull stdin from /dev/tty5 and redirect stdout and stderr to the pipes
+ * so we can log the output and put error messages into exceptions.
+ * We'll only use these messages should mount also return an error
+ * code.
*/
- fd = open("/dev/tty5", O_RDONLY);
+ tty_fd = open("/dev/tty5", O_RDONLY);
close(STDIN_FILENO);
- dup2(fd, STDIN_FILENO);
- close(fd);
+ dup2(tty_fd, STDIN_FILENO);
+ close(tty_fd);

close(STDOUT_FILENO);
- dup2(programLogFD, STDOUT_FILENO);
-
- dup2(pipefd[1], STDERR_FILENO);
+ dup2(stdout_pipe[1], STDOUT_FILENO);
+ close(STDERR_FILENO);
+ dup2(stderr_pipe[1], STDERR_FILENO);

if (mode == IMOUNT_MODE_MOUNT) {
if (opts) {
- fprintf(stdout, "Running... %s -n -t %s -o %s %s %s
",
+ logProgramMessage(INFO, "Running... %s -n -t %s -o %s %s %s",
cmd, fs, opts, device, where);
rc = execl(cmd, cmd,
"-n", "-t", fs, "-o", opts, device, where, NULL);
exit(1);
} else {
- fprintf(stdout, "Running... %s -n -t %s %s %s
",
+ logProgramMessage(INFO, "Running... %s -n -t %s %s %s",
cmd, fs, device, where);
rc = execl(cmd, cmd, "-n", "-t", fs, device, where, NULL);
exit(1);
}
} else if (mode == IMOUNT_MODE_UMOUNT) {
- fprintf(stdout, "Running... %s %s
", cmd, where);
+ logProgramMessage(INFO, "Running... %s %s", cmd, where);
rc = execl(cmd, cmd, where, NULL);
exit(1);
} else {
- fprintf(stdout, "Running... Unknown imount mode: %d
", mode);
+ logProgramMessage(ERROR, "Running... Unknown imount mode: %d
", mode);
exit(1);
}
}

- close(pipefd[1]);
+ close(stdout_pipe[1]);
+ close(stderr_pipe[1]);

- if (err != NULL) {
- if (*err != NULL) {
- rc = readFD(pipefd[0], err);
- rc = write(programLogFD, *err, 4096);
- }
+ char *buffer = NULL;
+ rc = readFD(stdout_pipe[0], &buffer);
+ if (rc > 0) {
+ rstrip(buffer);
+ logProgramMessage(INFO, buffer);
+ free(buffer);
+ buffer = NULL;
}
+ rc = readFD(stderr_pipe[0], &buffer);
+ if (rc > 0) {
+ rstrip(buffer);
+ logProgramMessage(ERROR, buffer);
+ if (err != NULL)
+ *err = buffer;
+ else
+ free(buffer);
+ }
+ close(stdout_pipe[0]);
+ close(stderr_pipe[0]);

- close(pipefd[0]);
waitpid(child, &status, 0);

- close(programLogFD);
-
if (opts) {
free(opts);
}
--
1.6.6

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

Hi,

> + rc = readFD(stdout_pipe[0], &buffer);
> + if (rc > 0) {
> + rstrip(buffer);
> + logProgramMessage(INFO, buffer);
> + free(buffer);
> + buffer = NULL;
> }
> + rc = readFD(stderr_pipe[0], &buffer);
> + if (rc > 0) {
> + rstrip(buffer);
> + logProgramMessage(ERROR, buffer);
> + if (err != NULL)
> + *err = buffer;
> + else
> + free(buffer);
> + }
> + close(stdout_pipe[0]);
> + close(stderr_pipe[0]);
>
> - close(pipefd[0]);
> waitpid(child, &status, 0);

Are you sure this is safe? What about the case when stderr gets filled by enough data to fill the kernel buffer for that pipe while you are waiting for stdout? In my experience the calling program then block until you read some data from the pipe.. but you are waiting for stdout so I think you could end up in deadlock here.

Martin

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

On 03/08/2010 11:03 AM, Martin Sivak wrote:

Hi,


+ rc = readFD(stdout_pipe[0],&buffer);
+ if (rc> 0) {
+ rstrip(buffer);
+ logProgramMessage(INFO, buffer);
+ free(buffer);
+ buffer = NULL;
}
+ rc = readFD(stderr_pipe[0],&buffer);
+ if (rc> 0) {
+ rstrip(buffer);
+ logProgramMessage(ERROR, buffer);
+ if (err != NULL)
+ *err = buffer;
+ else
+ free(buffer);
+ }
+ close(stdout_pipe[0]);
+ close(stderr_pipe[0]);

- close(pipefd[0]);
waitpid(child,&status, 0);


Are you sure this is safe? What about the case when stderr gets filled by enough data to fill the kernel buffer for that pipe while you are waiting for stdout? In my experience the calling program then block until you read some data from the pipe.. but you are waiting for stdout so I think you could end up in deadlock here.

Martin


Nice catch, this is something I did not consider. Looking at 'man 7
pipe' the capacity is 65kB so I will just assume there is not going to
be more than that written by mount to stderr or stdout. I'll add a
comment about it in the source code. If either problems appear or
there's more places we need this construction I am going to come up with
a general solution.


Ales

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@redhat.com
https://www.redhat.com/mailman/listinfo/anaconda-devel-list