Richard W.M. Jones wrote:
On Tue, Aug 11, 2009 at 05:20:37PM +0200, Jim Meyering wrote:
> Not useful from a testing standpoint (since this presumes
> a metric ton of other changes), but mostly a heads-up where I'm going.
> + char *r = strdup (template);
> + if (r == NULL) {
> + reply_with_perror ("strdup");
> + return NULL;
> + }
>
> CHROOT_IN;
> - r = mkdtemp (template);
> + r = mkdtemp (r);
> CHROOT_OUT;
>
> - if (r == NULL) {
> + if (r == NULL)
> reply_with_perror ("mkdtemp: %s", template);
> - return NULL;
> - }
>
> - /* The caller will free template AND try to free the return value,
> - * so we must make a copy here.
> - */
> - if (r == template) {
> - r = strdup (template);
> - if (r == NULL) {
> - reply_with_perror ("strdup");
> - return NULL;
> - }
> - }
> return r;
I think 'r' gets leaked along the error path (when mkdtemp returns NULL).
Good catch!
Here's the new version:
char *
do_mkdtemp (const char *template)
{
char *writable = strdup (template);
if (writable == NULL) {
reply_with_perror ("strdup");
return NULL;
}
CHROOT_IN;
char *r = mkdtemp (writable);
CHROOT_OUT;
if (r == NULL) {
reply_with_perror ("mkdtemp: %s", template);
free (writable);
}
return r;
}