Richard W.M. Jones wrote: ...
Subject: [PATCH 3/3] daemon: Fix hexdump to work on absolute symbolic links (RHBZ#579608). ...
This looks fine. Glad you caught the s/perror/reply_with_error/ problem you mentioned in the follow-up comment.
char * do_hexdump (const char *path) { - char *buf; - int r; + int fd, flags, r;
I have to say I would find it easier to read/review if each of these three declarations were moved down to its point of first use.
char *out, *err;
- buf = sysroot_path (path); - if (!buf) { - reply_with_perror ("malloc"); + CHROOT_IN; + fd = open (path, O_RDONLY); + CHROOT_OUT; + + if (fd == -1) { + perror (path); return NULL; }
- r = command (&out, &err, "hexdump", "-C", buf, NULL); - free (buf); + flags = COMMAND_FLAG_CHROOT_COPY_FILE_TO_STDIN | fd; + r = commandf (&out, &err, flags, "hexdump", "-C", NULL); if (r == -1) { reply_with_error ("%s: %s", path, err); free (err);