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

[Jim Meyering]

"Daniel P. Berrange" <berrange at 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.

And did you mean to include the TCP socket changes in this set?

> +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..