[PATCH] start-stop-daemon: make --exec follow symlinks.

Joakim Tjernlund Joakim.Tjernlund at transmode.se
Sun Apr 20 08:57:30 UTC 2008


> -----Original Message-----
> From: Denys Vlasenko [mailto:vda.linux at googlemail.com]
> Sent: den 20 april 2008 03:29
> To: Joakim Tjernlund
> Cc: busybox at busybox.net
> Subject: Re: [PATCH] start-stop-daemon: make --exec follow symlinks.
> 
> On Sunday 20 April 2008 00:16, Joakim Tjernlund wrote:
> > > Now xmalloc_open_read_close will return NULL if open fails,
> > > and this fixes the bug.
> >
> > Nope, read what I wrote:
> >
> > "hmm, xmalloc_open_read_close do use xopen though, but even if
> > this was fixed, it can die. The other users expects this too.
> > It does not make sense to fail open/lseek/read and return
> > a ptr with garbage in it. If you make sizep mandatory, it might make
> > sense if you do *sizep =-1 in case of error."
> >
> > You cannot die due to I/O errors, nor return a NULL to
> > start-stop-daemon. The function name itself implies this
> > but doesn't do so.
> 
> Awww I see now.
> 
> Fixed it. Also decided to go your way and use open_read_close
> (not xmalloc one). It is very slightly bigger but doesn't do
> so many malloc/realloc/free's.

But you do lots of xstat().
Moving only xstat() really is a bug waiting to happen as this
deviates from the rest of the option handling.

Secondly, as I mentioned several times, why not use fstat(),
something like:

diff --git a/libbb/read.c b/libbb/read.c
index e5f140f..1d52f83 100644
--- a/libbb/read.c
+++ b/libbb/read.c
@@ -211,22 +211,20 @@ void *xmalloc_open_read_close(const char *filename, size_t *sizep)
        size_t size;
        int fd;
        off_t len;
+       struct stat st;

        fd = open(filename, O_RDONLY);
        if (fd < 0)
                return NULL;

+       st.st_size = 0; /* in case fstat fail, define to 0 */
+       fstat(fd, &st);
        /* /proc/N/stat files report len 0 here */
        /* In order to make such files readable, we add small const */
-       size = 0x3ff; /* read only 1k on unseekable files */
-       len = lseek(fd, 0, SEEK_END) | 0x3ff; /* + up to 1k */
-       if (len != (off_t)-1) {
-               xlseek(fd, 0, SEEK_SET);
-               size = sizep ? *sizep : INT_MAX;
-               if (len < size)
-                       size = len;
-       }
-
+       len = st.st_size | 0x3ff; /* read only 1k on unseekable files */
+       size = sizep ? *sizep : INT_MAX;
+       if (len < size)
+               size = len;
        buf = xmalloc(size + 1);
        size = read_close(fd, buf, size);
        if ((ssize_t)size < 0) {

 Jocke

PS.
   I don't mind if you credit me in the commit logs.




More information about the busybox mailing list