[PATCH 2/2] source4/libcli: Only set ctemp set on success

[Sam Lang]

On Mon, Apr 22, 2013 at 10:03 AM, Volker Lendecke
<Volker.Lendecke at sernet.de> wrote:
> On Wed, Apr 17, 2013 at 10:56:35AM -0500, samlang at gmail.com wrote:
>> From: Sam Lang <sam.lang at inktank.com>
>>
>> If open fails ctemp.out.name probably won't be valid and strdup
>> will cause a segv.  Only set the path if open succeeds.
>>
>> Signed-off-by: Sam Lang <sam.lang at inktank.com>
>> ---
>>  source4/libcli/clifile.c |    6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/source4/libcli/clifile.c b/source4/libcli/clifile.c
>> index f5e02dd..6568de7 100644
>> --- a/source4/libcli/clifile.c
>> +++ b/source4/libcli/clifile.c
>> @@ -663,11 +663,11 @@ int smbcli_ctemp(struct smbcli_tree *tree, const char *path, char **tmp_path)
>>       open_parms.ctemp.in.write_time = 0;
>>
>>       status = smb_raw_open(tree, mem_ctx, &open_parms);
>> -     if (tmp_path) {
>> -             *tmp_path = strdup(open_parms.ctemp.out.name);
>> -     }
>>       talloc_free(mem_ctx);
>>       if (NT_STATUS_IS_OK(status)) {
>> +             if (tmp_path) {
>> +                     *tmp_path = strdup(open_parms.ctemp.out.name);
>> +             }
>
> Doesn't the talloc_free(mem_ctx) also have to be moved? From
> what I see open_parms.ctemp.out.name is a talloc child of
> it, so it will be freed before it's referenced.

Yep, you're right.  Sending an updated patch set (including changes
from Jeremy's comments) now.
-sam

>
> With best regards,
>
> Volker Lendecke
>
> --
> SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
> phone: +49-551-370000-0, fax: +49-551-370000-9
> AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
> http://www.sernet.de, mailto:kontakt at sernet.de