Samba 4 libnet_join and RPC-JOIN torture test

[Stefan (metze) Metzmacher]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Brad Henry schrieb:
> Stefan Metzmacher wrote:
> 
>> Am Montag, 12. September 2005 08:07 schrieb Andrew Bartlett:
>>  
>>
>>> On Mon, 2005-09-12 at 08:02 +0200, Stefan Metzmacher wrote:
>>>   
>>>
>>>> Am Montag, 12. September 2005 02:22 schrieb Brad Henry:
>>>>     
>>>>
>>>>> +       if (!lsa_open_policy.in.system_name) {
>>>>> +               r->out.error_string = NULL;
>>>>> +               talloc_free(tmp_ctx);
>>>>> +               return NT_STATUS_NO_MEMORY;
>>>>> +       }
>>>>>       
>>>>
>>>> please use if (!lsa_open_policy.in.system_name) goto no_mem;
>>>>
>>>> in all places, as you missed sometimes the talloc_free(tmp_ctx),
>>>>     
>>>
>>> Metze, I really don't like that style of code.  Yes, we should be
>>> consistent, and perhaps we should ensure that r->out.error_string = NULL
>>>   
>>
>>
>> yep, that's what we should do
>>  
>>
>>> is done at the top, but I think the goto is ugly and overboard.
>>>   
>>
>>
>> but the problem is the talloc_free(tmp_ctx), which needs to be done,
>> and in client side functions (like this libnet_ functions) we can't
>> just skip it.
>>
>>  
>>
>>> (The thing I most enjoy about the new talloc is that it killed most of
>>> that madness...)
>>>   
>>
>>
>> For server code where the whole in coming mem_ctx is destroyed, on
>> failure,
>> we use just NT_STATUS_HAVE_NO_MEMORY(string);
>>
>> this gives us a nice way for a just one line of error handling.
>>
>> maybe we should add a bunch of macros, which take also a ppointer to
>> the tmp_mem_ctx, and free's it before returning.
>>
>> some thing like this.
>>
>> NT_STATUS_HAVE_NO_MEMORY_TMPMEM(data, tmp_mem) do {
>>     if (!data) {
>>         if (tmp_mem) talloc_free(tmp_mem);
>>         return NT_STATUS_NO_MEMORY;
>>     }
>> } while (0)
>>
>> NT_STATUS_NOT_OK_RETURN_TMPMEM(status, tmp_mem) do {
>>     if (!NT_STATUS_IS_OK(status)) {
>>         if (tmp_mem) talloc_free(tmp_mem);
>>         return status;
>>     }
>> } while (0)
>>
>> then we would have a nice way of having just one line of error handling,
>> which makes the code much more readable, than having 4 or 5 lines of
>> error handling for one line of real code.
>>
>>  
>>
> So if these macro's existed, I could do something like
> 
> if (NT_STATUS_NO_MEMORY_TMPMEM(lsa_open_policy.in.system_name, tmp_ctx))
>        return NT_STATUS_NO_MEMORY;
> 
> and
> 
> if (NT_STATUS_NOT_OK_TMPMEM(status, tmp_ctx)) return status;
> 
> right?
> 
> I think that would be a lot nicer and more readable.  Should I add these
> two macros to source/include/nt_status.h  then and change my code to use
> them?

no just this:

str = talloc_asprintf(tmp_ctx,"Hello World");
NT_STATUS_NO_MEMORY_TMPMEM(str, tmp_ctx);

status = libnet_foo(ctx, tmp_ctx, &r);
NT_STATUS_NOT_OK_RETURN_TMPMEM(status, tmp_ctx);

abartlet: would you be happy with this?
(if so I'll add the macros)

- --
metze

Stefan Metzmacher <metze at samba.org> www.samba.org
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3-nr1 (Windows XP)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFDJmVZm70gjA5TCD8RAvwGAJ4tHCff34SXbo/pnAvz4eZoR0xCPwCgmA7D
hrs71L5jaRCpWjnFoXcjCsk=
=b+RC
-----END PGP SIGNATURE-----