Samba 4 libnet_join and RPC-JOIN torture test

Brad Henry j0j0 at riod.ca
Mon Sep 12 19:56:42 GMT 2005


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?

Thanks,
Brad




More information about the samba-technical mailing list