[SCM] Samba Shared Repository - branch master updated

[simo]

On Thu, 2011-12-29 at 01:12 +0100, Andrew Bartlett wrote:
> --- a/auth/kerberos/kerberos_pac.c
> +++ b/auth/kerberos/kerberos_pac.c
> @@ -77,7 +77,7 @@ krb5_error_code check_pac_checksum(TALLOC_CTX
> *mem_ctx,
>  *
>  * @return - A NTSTATUS error code
>  */
> -NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx,
> +NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx_out,
>                              DATA_BLOB pac_data_blob,
>                              krb5_context context,
>                              const krb5_keyblock *krbtgt_keyblock,
> @@ -109,13 +109,21 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX
> *mem_ctx,
>  
>         bool bool_ret;
>  
> -       *pac_data_out = NULL;
> +       TALLOC_CTX *mem_ctx = talloc_new(mem_ctx_out);
> +       if (!mem_ctx) {
> +               return NT_STATUS_NO_MEMORY;
> +       }
> + 

Andrew,
this may seem just a nitpick but I really find it utterly confusing to
change naming conventions for memory contexts in this way.

Why mem_ctx_out/mem_ctx instead of the customary mem_ctx/tmp_ctx ?

Also given you have to free mem_ctx all over for every return path why
not just change the code to do:
status = NT_ERROR_CODE;
goto done;

and then at then end just:

status = NT_STATUS_OK;

done:
talloc_free(tmp_ctx);
return status;

Both these customs makes the code more readable and less error prone.

Simo.


-- 
Simo Sorce
Samba Team GPL Compliance Officer <simo at samba.org>
Principal Software Engineer at Red Hat, Inc. <simo at redhat.com>