From 257b7b09298f7cb983b2f31b87fc5e46e0f62f0c Mon Sep 17 00:00:00 2001 From: Derrell Lipman Date: Thu, 28 Feb 2008 11:23:20 -0500 Subject: Initial revamp of the libsmbclient interface. The libsmbclient interface has suffered from difficulty of improvement and feature enrichment without causing ABI breakage. Although there were a number of issues, the primary ones were: (a) the user of the library would manually manipulate the context structure members, meaning that nothing in the context structure could change other than adding stuff at the end; (b) there were three methods of setting options: setting bits in a flags field within the context structure, setting explicit options variables within an options structure in the context structure, and by calling the smbc_option_set() function; (c) the authentication callback did not traditionally provide enough information to the callee which required adding an option for a callback with a different signature, and now there are requests for even more information at the callback, requiring yet a third signature and option to set it (if we implement that feature). This commit provides a reorganization of the code which fixes (a) and (b). The context structure is now entirely opaque, and there are setter and getter functions for manipulating it. This makes maintaining ABI consistency much, much easier. Additionally, the options setting/getting has been unified into a single mechanism using smbc_option_set() and smbc_option_get(). Yet to be completed is a refactoring of the authentication callback (c). The test programs in examples/libsmbclient have been modified (if necessary; some applications require no changes at all) for the new API and a few have been minimally tested. Derrell (This used to be commit d4b4bae8ded824d06ad5ab0e219f71187ee5c771) --- source3/libsmb/libsmb_cache.c | 74 +++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 42 deletions(-) (limited to 'source3/libsmb/libsmb_cache.c') diff --git a/source3/libsmb/libsmb_cache.c b/source3/libsmb/libsmb_cache.c index b98df024fa..e0571fa9fe 100644 --- a/source3/libsmb/libsmb_cache.c +++ b/source3/libsmb/libsmb_cache.c @@ -22,11 +22,8 @@ */ #include "includes.h" - -#include "include/libsmbclient.h" -#include "../include/libsmb_internal.h" - -int smbc_default_cache_functions(SMBCCTX * context); +#include "libsmbclient.h" +#include "libsmb_internal.h" /* * Structure we use if internal caching mechanism is used @@ -48,9 +45,13 @@ struct smbc_server_cache { * Add a new connection to the server cache. * This function is only used if the external cache is not enabled */ -static int smbc_add_cached_server(SMBCCTX * context, SMBCSRV * newsrv, - const char * server, const char * share, - const char * workgroup, const char * username) +int +SMBC_add_cached_server(SMBCCTX * context, + SMBCSRV * newsrv, + const char * server, + const char * share, + const char * workgroup, + const char * username) { struct smbc_server_cache * srvcache = NULL; @@ -88,7 +89,7 @@ static int smbc_add_cached_server(SMBCCTX * context, SMBCSRV * newsrv, goto failed; } - DLIST_ADD((context->server_cache), srvcache); + DLIST_ADD((context->cache.server_cache_data), srvcache); return 0; failed: @@ -108,13 +109,17 @@ static int smbc_add_cached_server(SMBCCTX * context, SMBCSRV * newsrv, * returns server handle on success, NULL on error (not found) * This function is only used if the external cache is not enabled */ -static SMBCSRV * smbc_get_cached_server(SMBCCTX * context, const char * server, - const char * share, const char * workgroup, const char * user) +SMBCSRV * +SMBC_get_cached_server(SMBCCTX * context, + const char * server, + const char * share, + const char * workgroup, + const char * user) { struct smbc_server_cache * srv = NULL; /* Search the cache lines */ - for (srv=((struct smbc_server_cache *)context->server_cache);srv;srv=srv->next) { + for (srv = context->cache.server_cache_data; srv; srv = srv->next) { if (strcmp(server,srv->server_name) == 0 && strcmp(workgroup,srv->workgroup) == 0 && @@ -146,7 +151,7 @@ static SMBCSRV * smbc_get_cached_server(SMBCCTX * context, const char * server, * a connection to the server (other than the * attribute server connection) is cool. */ - if (context->options.one_share_per_server) { + if (context->one_share_per_server) { /* * The currently connected share name * doesn't match the requested share, so @@ -156,7 +161,7 @@ static SMBCSRV * smbc_get_cached_server(SMBCCTX * context, const char * server, /* Sigh. Couldn't disconnect. */ cli_shutdown(srv->server->cli); srv->server->cli = NULL; - context->callbacks.remove_cached_srv_fn(context, srv->server); + context->cache.remove_cached_server_fn(context, srv->server); continue; } @@ -171,7 +176,7 @@ static SMBCSRV * smbc_get_cached_server(SMBCCTX * context, const char * server, /* Out of memory. */ cli_shutdown(srv->server->cli); srv->server->cli = NULL; - context->callbacks.remove_cached_srv_fn(context, srv->server); + context->cache.remove_cached_server_fn(context, srv->server); continue; } @@ -190,15 +195,17 @@ static SMBCSRV * smbc_get_cached_server(SMBCCTX * context, const char * server, * returns 0 on success * This function is only used if the external cache is not enabled */ -static int smbc_remove_cached_server(SMBCCTX * context, SMBCSRV * server) +int +SMBC_remove_cached_server(SMBCCTX * context, + SMBCSRV * server) { struct smbc_server_cache * srv = NULL; - for (srv=((struct smbc_server_cache *)context->server_cache);srv;srv=srv->next) { + for (srv = context->cache.server_cache_data; srv; srv = srv->next) { if (server == srv->server) { /* remove this sucker */ - DLIST_REMOVE(context->server_cache, srv); + DLIST_REMOVE(context->cache.server_cache_data, srv); SAFE_FREE(srv->server_name); SAFE_FREE(srv->share_name); SAFE_FREE(srv->workgroup); @@ -216,40 +223,23 @@ static int smbc_remove_cached_server(SMBCCTX * context, SMBCSRV * server) * Try to remove all the servers in cache * returns 1 on failure and 0 if all servers could be removed. */ -static int smbc_purge_cached(SMBCCTX * context) +int +SMBC_purge_cached_servers(SMBCCTX * context) { struct smbc_server_cache * srv; struct smbc_server_cache * next; int could_not_purge_all = 0; - for (srv = ((struct smbc_server_cache *) context->server_cache), - next = (srv ? srv->next :NULL); + for (srv = context->cache.server_cache_data, + next = (srv ? srv->next :NULL); srv; - srv = next, next = (srv ? srv->next : NULL)) { + srv = next, + next = (srv ? srv->next : NULL)) { - if (smbc_remove_unused_server(context, srv->server)) { + if (SMBC_remove_unused_server(context, srv->server)) { /* could not be removed */ could_not_purge_all = 1; } } return could_not_purge_all; } - - - -/* - * This functions initializes all server-cache related functions - * to the default (internal) system. - * - * We use this to make the rest of the cache system static. - */ - -int smbc_default_cache_functions(SMBCCTX * context) -{ - context->callbacks.add_cached_srv_fn = smbc_add_cached_server; - context->callbacks.get_cached_srv_fn = smbc_get_cached_server; - context->callbacks.remove_cached_srv_fn = smbc_remove_cached_server; - context->callbacks.purge_cached_fn = smbc_purge_cached; - - return 0; -} -- cgit From 4ba42cbe0f6bbd25848786e1a87c06aca79b98ea Mon Sep 17 00:00:00 2001 From: Derrell Lipman Date: Fri, 29 Feb 2008 13:34:35 -0500 Subject: Modified revamp of the libsmbclient interface. Given the tacit (if that) approval by some people, and clear disapproval by others for my proposed clean-up and reorganization of libsmbclient, I've come up with a slightly different approach. This commit changes back to the original libsmbclient.h SMBCCTX structure which will maintain ABI compatibility. I retain, here, the setter and getter functions which all new code should use. Older programs already compiled should continue to work fine. Older programs being recompiled will encounter compile-time errors (intentionally!) so that the code can be corrected to use the setter/getter interfaces. Although this doesn't clean up the interface in the way I had wanted, the code reorganization and requirement for new programs to use the setters and getters allows future progress to be made on libsmbclient without further muddying up the interface, while retaining the ABI compatibility that was the big issue causing disapproval. I hope that this compromise is adequate. Derrell (This used to be commit 56429a3d60b2a48963342f6340b3c01469a892c6) --- source3/libsmb/libsmb_cache.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'source3/libsmb/libsmb_cache.c') diff --git a/source3/libsmb/libsmb_cache.c b/source3/libsmb/libsmb_cache.c index e0571fa9fe..7ff92f1b4e 100644 --- a/source3/libsmb/libsmb_cache.c +++ b/source3/libsmb/libsmb_cache.c @@ -151,7 +151,7 @@ SMBC_get_cached_server(SMBCCTX * context, * a connection to the server (other than the * attribute server connection) is cool. */ - if (context->one_share_per_server) { + if (context->options.one_share_per_server) { /* * The currently connected share name * doesn't match the requested share, so -- cgit From 223940d9a887c5b98a5c873797302a6a9407ad7f Mon Sep 17 00:00:00 2001 From: Derrell Lipman Date: Sat, 1 Mar 2008 20:44:21 -0500 Subject: Additional revamped libsmbclient documentation - Ensured that all public functions have documentation in libsmbclient.h - Reformatted for "proper" indentation - Re-added temporarily-disabled alternate authentication function capability Derrell (This used to be commit 64b7150d92849a1e1e2416b9dcc12fae8d6bea99) --- source3/libsmb/libsmb_cache.c | 54 +++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 27 deletions(-) (limited to 'source3/libsmb/libsmb_cache.c') diff --git a/source3/libsmb/libsmb_cache.c b/source3/libsmb/libsmb_cache.c index 7ff92f1b4e..ff13fd7eac 100644 --- a/source3/libsmb/libsmb_cache.c +++ b/source3/libsmb/libsmb_cache.c @@ -35,10 +35,10 @@ struct smbc_server_cache { char *workgroup; char *username; SMBCSRV *server; - + struct smbc_server_cache *next, *prev; }; - + /* @@ -54,51 +54,51 @@ SMBC_add_cached_server(SMBCCTX * context, const char * username) { struct smbc_server_cache * srvcache = NULL; - + if (!(srvcache = SMB_MALLOC_P(struct smbc_server_cache))) { errno = ENOMEM; DEBUG(3, ("Not enough space for server cache allocation\n")); return 1; } - + ZERO_STRUCTP(srvcache); - + srvcache->server = newsrv; - + srvcache->server_name = SMB_STRDUP(server); if (!srvcache->server_name) { errno = ENOMEM; goto failed; } - + srvcache->share_name = SMB_STRDUP(share); if (!srvcache->share_name) { errno = ENOMEM; goto failed; } - + srvcache->workgroup = SMB_STRDUP(workgroup); if (!srvcache->workgroup) { errno = ENOMEM; goto failed; } - + srvcache->username = SMB_STRDUP(username); if (!srvcache->username) { errno = ENOMEM; goto failed; } - + DLIST_ADD((context->cache.server_cache_data), srvcache); return 0; - - failed: + +failed: SAFE_FREE(srvcache->server_name); SAFE_FREE(srvcache->share_name); SAFE_FREE(srvcache->workgroup); SAFE_FREE(srvcache->username); SAFE_FREE(srvcache); - + return 1; } @@ -117,19 +117,19 @@ SMBC_get_cached_server(SMBCCTX * context, const char * user) { struct smbc_server_cache * srv = NULL; - + /* Search the cache lines */ for (srv = context->cache.server_cache_data; srv; srv = srv->next) { - + if (strcmp(server,srv->server_name) == 0 && strcmp(workgroup,srv->workgroup) == 0 && strcmp(user, srv->username) == 0) { - + /* If the share name matches, we're cool */ if (strcmp(share, srv->share_name) == 0) { return srv->server; } - + /* * We only return an empty share name or the attribute * server on an exact match (which would have been @@ -137,7 +137,7 @@ SMBC_get_cached_server(SMBCCTX * context, */ if (*share == '\0' || strcmp(share, "*IPC$") == 0) continue; - + /* * Never return an empty share name or the attribute * server if it wasn't what was requested. @@ -145,7 +145,7 @@ SMBC_get_cached_server(SMBCCTX * context, if (*srv->share_name == '\0' || strcmp(srv->share_name, "*IPC$") == 0) continue; - + /* * If we're only allowing one share per server, then * a connection to the server (other than the @@ -164,7 +164,7 @@ SMBC_get_cached_server(SMBCCTX * context, context->cache.remove_cached_server_fn(context, srv->server); continue; } - + /* * Save the new share name. We've * disconnected from the old share, and are @@ -179,13 +179,13 @@ SMBC_get_cached_server(SMBCCTX * context, context->cache.remove_cached_server_fn(context, srv->server); continue; } - - + + return srv->server; } } } - + return NULL; } @@ -200,10 +200,10 @@ SMBC_remove_cached_server(SMBCCTX * context, SMBCSRV * server) { struct smbc_server_cache * srv = NULL; - + for (srv = context->cache.server_cache_data; srv; srv = srv->next) { if (server == srv->server) { - + /* remove this sucker */ DLIST_REMOVE(context->cache.server_cache_data, srv); SAFE_FREE(srv->server_name); @@ -229,13 +229,13 @@ SMBC_purge_cached_servers(SMBCCTX * context) struct smbc_server_cache * srv; struct smbc_server_cache * next; int could_not_purge_all = 0; - + for (srv = context->cache.server_cache_data, next = (srv ? srv->next :NULL); srv; srv = next, next = (srv ? srv->next : NULL)) { - + if (SMBC_remove_unused_server(context, srv->server)) { /* could not be removed */ could_not_purge_all = 1; -- cgit From fc65c6698e74e40828e218dc2e6de0c094bfe6e0 Mon Sep 17 00:00:00 2001 From: Derrell Lipman Date: Sat, 1 Mar 2008 21:19:52 -0500 Subject: Return NULL, not 0, from a function which returns a pointer. (This used to be commit 23cb9c49e3724cecaa66655ef64c3111bf14c552) --- source3/libsmb/libsmb_cache.c | 1 - 1 file changed, 1 deletion(-) (limited to 'source3/libsmb/libsmb_cache.c') diff --git a/source3/libsmb/libsmb_cache.c b/source3/libsmb/libsmb_cache.c index ff13fd7eac..c45aba4544 100644 --- a/source3/libsmb/libsmb_cache.c +++ b/source3/libsmb/libsmb_cache.c @@ -1,4 +1,3 @@ - /* Unix SMB/CIFS implementation. SMB client library implementation (server cache) -- cgit From 1363ee9d65965704f716965c6cbcfc0693ca2401 Mon Sep 17 00:00:00 2001 From: Derrell Lipman Date: Mon, 3 Mar 2008 18:13:33 -0500 Subject: Continued revamping of libsmbclient. - James suggested using gcc's "deprecated" attribute to mark the context structure fields to generate warnings. This creates a scenario with the best of all worlds. I'm able to move to an organization that more easily allows future enhancements, while avoiding any mandatory changes by applications. Thanks, James! - Updated WHATSNEW.txt so that it accurately reflects the current state of affairs. Derrell (This used to be commit a67f96fbe9683b46c2149f7cb439d13f7f0e6ecd) --- source3/libsmb/libsmb_cache.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'source3/libsmb/libsmb_cache.c') diff --git a/source3/libsmb/libsmb_cache.c b/source3/libsmb/libsmb_cache.c index c45aba4544..bfacea368d 100644 --- a/source3/libsmb/libsmb_cache.c +++ b/source3/libsmb/libsmb_cache.c @@ -88,7 +88,7 @@ SMBC_add_cached_server(SMBCCTX * context, goto failed; } - DLIST_ADD((context->cache.server_cache_data), srvcache); + DLIST_ADD(context->internal->server_cache, srvcache); return 0; failed: @@ -118,7 +118,7 @@ SMBC_get_cached_server(SMBCCTX * context, struct smbc_server_cache * srv = NULL; /* Search the cache lines */ - for (srv = context->cache.server_cache_data; srv; srv = srv->next) { + for (srv = context->internal->server_cache; srv; srv = srv->next) { if (strcmp(server,srv->server_name) == 0 && strcmp(workgroup,srv->workgroup) == 0 && @@ -150,7 +150,7 @@ SMBC_get_cached_server(SMBCCTX * context, * a connection to the server (other than the * attribute server connection) is cool. */ - if (context->options.one_share_per_server) { + if (smbc_getOptionOneSharePerServer(context)) { /* * The currently connected share name * doesn't match the requested share, so @@ -160,7 +160,7 @@ SMBC_get_cached_server(SMBCCTX * context, /* Sigh. Couldn't disconnect. */ cli_shutdown(srv->server->cli); srv->server->cli = NULL; - context->cache.remove_cached_server_fn(context, srv->server); + smbc_getFunctionRemoveCachedServer(context)(context, srv->server); continue; } @@ -175,7 +175,7 @@ SMBC_get_cached_server(SMBCCTX * context, /* Out of memory. */ cli_shutdown(srv->server->cli); srv->server->cli = NULL; - context->cache.remove_cached_server_fn(context, srv->server); + smbc_getFunctionRemoveCachedServer(context)(context, srv->server); continue; } @@ -200,11 +200,11 @@ SMBC_remove_cached_server(SMBCCTX * context, { struct smbc_server_cache * srv = NULL; - for (srv = context->cache.server_cache_data; srv; srv = srv->next) { + for (srv = context->internal->server_cache; srv; srv = srv->next) { if (server == srv->server) { /* remove this sucker */ - DLIST_REMOVE(context->cache.server_cache_data, srv); + DLIST_REMOVE(context->internal->server_cache, srv); SAFE_FREE(srv->server_name); SAFE_FREE(srv->share_name); SAFE_FREE(srv->workgroup); @@ -229,7 +229,7 @@ SMBC_purge_cached_servers(SMBCCTX * context) struct smbc_server_cache * next; int could_not_purge_all = 0; - for (srv = context->cache.server_cache_data, + for (srv = context->internal->server_cache, next = (srv ? srv->next :NULL); srv; srv = next, -- cgit