[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] PATCH: 19/25: Remove use of non-threadsafe POSIX apis



On Thu, Jan 22, 2009 at 04:21:25PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange redhat com> wrote:
> > On Fri, Jan 16, 2009 at 12:25:02PM +0000, Daniel P. Berrange wrote:
> >> On Fri, Jan 16, 2009 at 12:21:59PM +0000, Richard W.M. Jones wrote:
> >> > On Tue, Jan 13, 2009 at 05:46:37PM +0000, Daniel P. Berrange wrote:
> >> > > +        char buf[1024];
> >> >
> >> > sysconf (_SC_GETPW_R_SIZE_MAX)?
> >> >
> >> > Looking at glibc's implementation of getpwuid (which uses getpwuid_r),
> >> > I see that glibc dynamically reallocates the buffer as necessary to
> >> > the correct size for the return value.  The logic of this is fairly
> >> > simple so maybe we should do the same?
> >> >
> >> > From glibc:
> >> >
> >> >   buffer = malloc (/*some_initial_size*/);
> >> >
> >> >   while (buffer != NULL
> >> >          && (getpwuid_r (const char *name, &resbuf, buffer,
> >> >                          buffer_size, &result)
> >> >              == ERANGE))
> >> >     {
> >> >       char *new_buf;
> >> >       buffer_size *= 2;
> >> >       new_buf = (char *) realloc (buffer, buffer_size);
> >> >       if (new_buf == NULL)
> >> >         {
> >> >           free (buffer);
> >> >           errno = ENOMEM;
> >> >         }
> >> >       buffer = new_buf;
> >> >     }
> >> >
> >> >
> >> > Anyhow, +1 but I'd be happier if these functions were centralized in
> >> > somewhere like src/utils.c.
> >>
> >> That's a good idea - in all the cases where we currently use getpwuid
> >> all we actually want is the home directory path. So we could add a
> >> simple func:
> >>
> >>    char *virUserHomeDirectory(uid_t uid);
> >>
> >> and hide all the horrific code in there.
> >
> > Here's an update with that usage in it
> 
> One problem in virGetUserDirectory.
> It always returns NULL.

Yep, already fixed that.
 
> And did you mean to include the TCP socket changes in this set?

Yes, its require to remove the use of non-threadsafe gethostname()
call

> > +char *virGetUserDirectory(virConnectPtr conn,
> > +                          uid_t uid)
> > +{
> > +    char *strbuf;
> > +    char *ret;
> > +    struct passwd pwbuf;
> > +    struct passwd *pw;
> > +    size_t strbuflen = sysconf(_SC_GETPW_R_SIZE_MAX);
> > +
> > +    if (VIR_ALLOC_N(strbuf, strbuflen) < 0) {
> > +        virReportOOMError(conn);
> > +        return NULL;
> > +    }
> > +
> > +    if (getpwuid_r(uid, &pwbuf, strbuf, strbuflen, &pw) != 0) {
> > +        virReportSystemError(conn, errno,
> > +                             _("Failed to find user record for uid '%d'"),
> > +                             uid);
> > +        VIR_FREE(strbuf);
> > +        return NULL;
> > +    }
> > +
> > +    ret = strdup(pw->pw_dir);
> > +
> > +    VIR_FREE(strbuf);
> > +    if (!ret)
> > +        virReportOOMError(conn);
> > +
> > +    return NULL;
> 
> This fucntion alwasy returns NULL..

-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]