Proposal: libsmbclient API

Derrell Lipman derrell.lipman at unwireduniverse.com
Sat Mar 28 23:03:46 GMT 2009


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.
>

Hi Andreas,

I've been researching this and I think the solution we've been discussion is
inadequate. The problem is that in many cases, errno gets set down in the
bowels of samba core code, particularly by the C library as called by the
functions in lib/system.c and lib/select.c. Just having
libsmbclient-specific code set the errno value in a context variable isn't
going to solve the problem. Any time errno is passed up from the "primitive"
(sys_) functions, there's the possibility that a thread switch can overwrite
its value. What's needed is a more general solution that protects the
critical sections of code in the primitives where errno can be set/modified,
and saves the errno value in thread-specific memory before releasing the
lock on the critical section. This is similar to what the OpenSSL locking
code does. Whatever implementation we use can be added to both the primitive
functions and to the libsmbclient code (i.e. wherever errno is possibly set
and expected to live unaltered). Particularly in the primitive functions,
the implementation should operate as it does currently if no critical
section handlers have been provided (as will likely be the case in smbd
which probably doesn't require them).

More below...

===
>
> What about a better version function and some macros?
>
> const char *smbc_version(int req_version_num);
>
> Which returns the version number if we match it or pass 0 to the function.
> On
> error return NULL.
>
> if (smbc_version(CSYNC_VERSION_INT(0,0,1) == NULL) {
>         fprintf(stderr, "Runtime version of libsmbclient too old!\n");
>   exit(1);
> }
>
> if (debug) {
>   printf ("libsmbclient %s\n", smbc_version(0));
> }
>
> And the string could look something like:
> 0.0.1
> 0.0.1/heimdal
> 0.0.1/MIT
> 0.0.1/heimdal/vendor
>

So it occurs to me that if I were using these functions, I'd want to know
not what version of the library is available, but more likely, whether the
version of the library is AT LEAST up to the version that supports the
features I want to use. This allows an application to continue to run even
if the library is upgraded, because the new library version will be greater
than the one the application expected, and that's still ok.

To implement versioning based on "feature availability", I can see a couple
of options. One option is to pass a second parameter to smbc_version which
indicates the comparison type to use:

enum SMBC_Comparison
{
  SMBC_Comparison_Less,
  SMBC_Comparison_LessEqual,
  SMBC_Comparison_Equal,
  SMBC_Comparison_GreaterEqual,
  SMBC_Comparison_Greater
};
bool smbc_version(unsigned int version, enum SMBC_Comparison compareType);

Another solution is to provide a read-only version function:

char * smbc_version(void);

and a separate function to determine if a feature is available:

bool smbc_featureAvailable(char * pFeatureName);

The former is more general but requires that the application know about what
features were added in what version. The latter makes it very easy to use a
feature if it happens to be available.

I don't have a strong preference for one option over the other, although I
think I lean towards the latter pair of functions over the enum version.

Thoughts?

Derrell


More information about the samba-technical mailing list