Proposal: libsmbclient API

[Derrell Lipman]

On Thu, Mar 26, 2009 at 12:52 PM, Andreas Schneider <mail at cynapses.org>wrote:

> On Thursday 26 March 2009 17:33:12 you wrote:
> > I haven't gone back over all of the code recently to see what, if
> anything,
> > remains other than the errno issues. Other than in libsmb_compat (the
> POSIX
> > compatibility layer), I don't believe there are any global variables or
> > anything else (other than errno) that should prevent libsmbclient from
> > being thread-safe, but the library really needs a thorough inspection to
> > ensure that.
>
> I will look at the libsmbclient code in the next days.
>
> > To start with, yes, I think it would be a good idea to add the
> > smbc_[sg]et_errno() function (which sets errno to the specified value for
> > backward compatibility in addition to storing it in the private area of
> the
> > context) and change libsmb_*.c to call smbc_set_errno() instead of
> > assigning to errno itself.
>
> Ok, I will implement this.
>

Great!


> What about a better version function and some macros?
>
Sounds fine.

>
> const char *smbc_version(int req_version_num);
>
The definition of that int parameter needs to be clear, which can be done
via function documentation, but I wonder if it makes more sense to pass
three separate version parts rather than a single integer? Or maybe a
structure parameter with structure members for each of the three parts?

>
> Which returns the version number if we match it or pass 0 to the function.
> On
> error return NULL.
>
If it's going to return a string, then how about just passing a string as
the parameter? It's easy enough to parse, and that way the interface is
consistent.

>
> if (smbc_version(CSYNC_VERSION_INT(0,0,1) == NULL) {
>         fprintf(stderr, "Runtime version of libsmbclient too old!\n");
>   exit(1);
> }
>
CSYNC_VERSION_INT looks like a macro from some other package but I don't
think I've ever seen it before. If the parameter does end up as a single
integer, I'd think it'd make sense for there to be a SMBC_VERSION_INT macro.

I'm not wedded to any of these suggestions I made; rather, they're just to
ensure we think through it before committing to an interface that'll be
around for a long time.

Cheers,

Derrell