[PATCH] make libsmbclient work with DFS (Was: [RFC] To make libsmbclient work for DFS shares for samba3)

[Derrell Lipman]

On Fri, Feb 20, 2009 at 7:08 PM, Jeremy Allison <jra at samba.org> wrote:

> On Thu, Feb 19, 2009 at 10:36:26AM -0500, Derrell Lipman wrote:
> > 2009/2/19 boyang <boyang at suse.de>
> >
> >     Hi, Derrell & Jeremy:
> >         This is the updated version of the patch. There are two
> >     improvements according to Derrell's comment:
> >     * Keep the smbc_set_credentials() API untact.
> >     * Merge smbc_set_crendentials_wrapper() into libsmb_path.c and
> >     libsmb_server.c
> >          Tested and worked fine for me. :-)
> >          Pls review it, Thanks!
> >
> >
> > Ok, I'm much happier with this patch. :-)  I have one change I think we
> should
> > make, for consistency, and a naming change request.
> >
> > 1. The function smbc_set_credentials_wrapper() takes an SMBCCTX * as a
> > parameter. The convention in the library is that functions that take a
> SMBCCTX
> > * parameter have it as the first parameter, so please move it from the
> end of
> > the parameter list to the beginning.
> >
> > 2. If smbc_set_credentials_wrapper() really should be an exposed function
> in
> > the the interface (and it looks like it probably should be), we can
> probably
> > come up with a better name for it. How about either
> smbc_set_credentials_auto()
> > or smbc_set_credentials_with_fallback() ?
>
> Sorry for not getting to this sooner...
>
> We really should make smbc_set_credentials_with_fallback()
> take all const values for workgroup, user, password. We
> don't want to be adding new non-const function signatures
> here.
>
> smbc_set_credentials should also have all const values
> as well, but that would be an ABI change which we're
> trying to avoid.
>
> But for new functions this should be const-only.
>

Hi Jeremy,

I have no problem making those parameters const, but it shouldn't make any
difference. In fact, we can change the parameters in smbc_set_credentials()
to const too without changing the ABI. Adding const to the parameters is
simply a debugging tool to ensure that we don't inadvertently write to the
user's buffers (which may not be writable). It's allowable to pass a
non-const parameter to a function that expects a const. It's just not
allowable to pass a const parameter to a function that does not specify
const since the function then has the option of writing to that buffer.
Therefore if we had declared it originally const and then considered
removing the const in the function definition and having our function write
to the pointed-to buffer, we'd have a problem, because then we couldn't be
sure that applications were passing writable buffers.

In short, adding const to these parameters only prevents the future
developer of these functions from messing with callers' buffers. It has no
effect now (as long as we're not already scribbling on our callers' buffers)
and it has no effect on the callers themselves going in the direction of no
const to specifying const.

Derrell