[SCM] Samba Shared Repository - branch v3-2-test updated - initial-v3-2-unstable-419-g9d0034f

[Derrell Lipman]

On Nov 30, 2007 5:20 PM, Jeremy Allison <jra at samba.org> wrote:
> The branch, v3-2-test has been updated
>       via  9d0034faed939a4534637696f1631ac2da60e4a3 (commit)
>      from  fa8115f32bfd37f75c284ff0f6906dbc2af0f40c (commit)
>
> http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-2-test
>
>
> - Log -----------------------------------------------------------------
> commit 9d0034faed939a4534637696f1631ac2da60e4a3
> Author: Jeremy Allison <jra at samba.org>
> Date:   Fri Nov 30 14:19:55 2007 -0800
>
>    Removed all pstrings from libsmbclient. Derryl please
>    check. Passes valgrind tests I've run in examples/libsmbclient.

Jeremy, I'm looking only at the patch right now since I don't have
easy access to the source tree.  In the patch, I see only two areas
that may warrant further verification:

> @@ -2744,7 +2743,14 @@ smbc_opendir_ctx(SMBCCTX *context,
>                 return NULL;
>         }
>
> -       if (user[0] == (char)0) fstrcpy(user, context->user);
> +       if (!user || user[0] == (char)0) {
> +               user = talloc_strdup(frame, context->user);
> +               if (!user) {
> +                       errno = ENOMEM;
> +                       TALLOC_FREE(frame);
> +                       return NULL;
> +               }
> +       }

I don't know how user[0] can become a null character, but if for some
reason 'user' is non-null but user[0] is a null character, the if
statement will be entered and user will be overwritten.  It's on
context 'frame' so maybe that's ok, but it'd probably be cleaner if
user were deallocated before being allocated if it was non-null.

> @@ -2847,8 +2853,13 @@ smbc_opendir_ctx(SMBCCTX *context,
>                        if ( !cli )
>                                continue;
>
> -                       pstrcpy(workgroup, wg_ptr);
> -                        fstrcpy(server, cli->desthost);
> +                       workgroup = talloc_strdup(frame, wg_ptr);
> +                       server = talloc_strdup(frame, cli->desthost);
> +                       if (!workgroup || !server) {
> +                               errno = ENOMEM;
> +                               TALLOC_FREE(frame);
> +                               return NULL;
> +                       }

This is newly-added clean-up (error handling) code where none was
previously needed.  Are we sure that's all the cleanup required before
returning?


Due to other constraints, I won't be back to being able to do any
Samba work until mid-December.  If further investigation in the source
is still required, that's when I'll be able to do it.

Cheers,

Derrell