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