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

"Daniel P. Berrange" <berrange(a)redhat.com&gt; 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..