From 376ae2e2a768d3db7a00b17924e8b5740014d1de Mon Sep 17 00:00:00 2001 From: Elizabeth Myers Date: Tue, 5 Apr 2016 03:30:02 -0500 Subject: [PATCH] Clean up the provider status logic. Provider status (done, running, not run) is now attached to the provider-specific data of the client. A reference count of auth instances is kept in the auth_client struct to determine if a client is done or not. This also moves a lot of the logic for manipulating provider data into into the provider.h header for inlining (no point in a function call for these simple accessors). --- authd/provider.c | 52 +++---------------- authd/provider.h | 99 ++++++++++++++++++++++++++++--------- authd/providers/blacklist.c | 2 +- authd/providers/ident.c | 2 +- authd/providers/opm.c | 2 +- authd/providers/rdns.c | 2 +- 6 files changed, 89 insertions(+), 70 deletions(-) diff --git a/authd/provider.c b/authd/provider.c index 58294488..c9c7b2c4 100644 --- a/authd/provider.c +++ b/authd/provider.c @@ -159,7 +159,7 @@ cancel_providers(struct auth_client *auth) { provider = ptr->data; - if(provider->cancel && is_provider_on(auth, provider->id)) + if(provider->cancel && is_provider_running(auth, provider->id)) /* Cancel if required */ provider->cancel(auth); } @@ -176,13 +176,12 @@ provider_done(struct auth_client *auth, provider_t id) rb_dlink_node *ptr; struct auth_provider *provider; - set_provider_off(auth, id); set_provider_done(auth, id); - if(!auth->providers) + if(!auth->refcount) { if(!auth->providers_starting) - /* Only do this when there are no providers left */ + /* Providers aren't being executed and refcount is 0, accept */ accept_client(auth, -1); return; } @@ -191,7 +190,7 @@ provider_done(struct auth_client *auth, provider_t id) { provider = ptr->data; - if(provider->completed != NULL && is_provider_on(auth, provider->id)) + if(provider->completed != NULL && is_provider_running(auth, provider->id)) /* Notify pending clients who asked for it */ provider->completed(auth, id); } @@ -237,7 +236,7 @@ reject_client(struct auth_client *auth, provider_t id, const char *data, const c */ rb_helper_write(authd_helper, "R %x %c %s %s %s :%s", auth->cid, reject, auth->username, auth->hostname, data, buf); - set_provider_off(auth, id); + set_provider_done(auth, id); cancel_providers(auth); } @@ -247,7 +246,7 @@ accept_client(struct auth_client *auth, provider_t id) { rb_helper_write(authd_helper, "A %x %s %s", auth->cid, auth->username, auth->hostname); - set_provider_off(auth, id); + set_provider_done(auth, id); cancel_providers(auth); } @@ -307,7 +306,7 @@ start_auth(const char *cid, const char *l_ip, const char *l_port, const char *c_ auth->providers_starting = false; /* If no providers are running, accept the client */ - if(!auth->providers) + if(!auth->refcount) accept_client(auth, -1); } @@ -368,7 +367,7 @@ provider_timeout_event(void *notused __unused) struct auth_provider *provider = ptr->data; const time_t timeout = get_provider_timeout(auth, provider->id); - if(is_provider_on(auth, provider->id) && provider->timeout != NULL && + if(is_provider_running(auth, provider->id) && provider->timeout != NULL && timeout > 0 && timeout < curtime) { provider->timeout(auth); @@ -376,38 +375,3 @@ provider_timeout_event(void *notused __unused) } } } - -void * -get_provider_data(struct auth_client *auth, uint32_t id) -{ - lrb_assert(id < rb_dlink_list_length(&auth_providers)); - return auth->data[id].data; -} - -void -set_provider_data(struct auth_client *auth, uint32_t id, void *data) -{ - lrb_assert(id < rb_dlink_list_length(&auth_providers)); - auth->data[id].data = data; -} - -void -set_provider_timeout_relative(struct auth_client *auth, uint32_t id, time_t timeout) -{ - lrb_assert(id < rb_dlink_list_length(&auth_providers)); - auth->data[id].timeout = timeout + rb_current_time(); -} - -void -set_provider_timeout_absolute(struct auth_client *auth, uint32_t id, time_t timeout) -{ - lrb_assert(id < rb_dlink_list_length(&auth_providers)); - auth->data[id].timeout = timeout; -} - -time_t -get_provider_timeout(struct auth_client *auth, uint32_t id) -{ - lrb_assert(id < rb_dlink_list_length(&auth_providers)); - return auth->data[id].timeout; -} diff --git a/authd/provider.h b/authd/provider.h index 3e57fa18..b04cf009 100644 --- a/authd/provider.h +++ b/authd/provider.h @@ -36,10 +36,18 @@ typedef enum PROVIDER_OPM, } provider_t; +typedef enum +{ + PROVIDER_STATUS_NOTRUN = 0, + PROVIDER_STATUS_RUNNING, + PROVIDER_STATUS_DONE, +} provider_status_t; + struct auth_client_data { - time_t timeout; /* Provider timeout */ - void *data; /* Proivder data */ + time_t timeout; /* Provider timeout */ + void *data; /* Provider data */ + provider_status_t status; /* Provider status */ }; struct auth_client @@ -57,10 +65,8 @@ struct auth_client char hostname[HOSTLEN + 1]; /* Used for DNS lookup */ char username[USERLEN + 1]; /* Used for ident lookup */ - uint32_t providers; /* Providers at work, - * none left when set to 0 */ - uint32_t providers_done; /* Providers completed */ bool providers_starting; /* Providers are still warming up */ + unsigned int refcount; /* Held references */ struct auth_client_data *data; /* Provider-specific data */ }; @@ -120,44 +126,93 @@ void reject_client(struct auth_client *auth, provider_t id, const char *data, co void handle_new_connection(int parc, char *parv[]); void handle_cancel_connection(int parc, char *parv[]); -void *get_provider_data(struct auth_client *auth, uint32_t id); -void set_provider_data(struct auth_client *auth, uint32_t id, void *data); -void set_provider_timeout_relative(struct auth_client *auth, uint32_t id, time_t timeout); -void set_provider_timeout_absolute(struct auth_client *auth, uint32_t id, time_t timeout); -time_t get_provider_timeout(struct auth_client *auth, uint32_t id); -/* Provider is operating on this auth_client (set this if you have async work to do) */ -static inline void -set_provider_on(struct auth_client *auth, provider_t provider) +/* Get a provider's raw status */ +static inline provider_status_t +get_provider_status(struct auth_client *auth, provider_t provider) { - auth->providers |= (1 << provider); + return auth->data[provider].status; } -/* Provider is no longer operating on this auth client (you should use provider_done) */ +/* Set a provider's raw status */ static inline void -set_provider_off(struct auth_client *auth, provider_t provider) +set_provider_status(struct auth_client *auth, provider_t provider, provider_status_t status) { - auth->providers &= ~(1 << provider); + auth->data[provider].status = status; } -/* Set the provider to done (you should use provider_done) */ +/* Set the provider as running + * If you're doing asynchronous work call this */ +static inline void +set_provider_running(struct auth_client *auth, provider_t provider) +{ + auth->refcount++; + set_provider_status(auth, provider, PROVIDER_STATUS_RUNNING); +} + +/* Provider is no longer operating on this auth client + * You should use provider_done and not this */ static inline void set_provider_done(struct auth_client *auth, provider_t provider) { - auth->providers_done |= (1 << provider); + auth->refcount--; + set_provider_status(auth, provider, PROVIDER_STATUS_DONE); } /* Check if provider is operating on this auth client */ static inline bool -is_provider_on(struct auth_client *auth, provider_t provider) +is_provider_running(struct auth_client *auth, provider_t provider) { - return auth->providers & (1 << provider); + return get_provider_status(auth, provider) == PROVIDER_STATUS_RUNNING; } +/* Check if provider has finished on this client */ static inline bool is_provider_done(struct auth_client *auth, provider_t provider) { - return auth->providers_done & (1 << provider); + return get_provider_status(auth, provider) == PROVIDER_STATUS_DONE; +} + +/* Get provider auth client data */ +static inline void * +get_provider_data(struct auth_client *auth, uint32_t id) +{ + lrb_assert(id < rb_dlink_list_length(&auth_providers)); + return auth->data[id].data; +} + +/* Set provider auth client data */ +static inline void +set_provider_data(struct auth_client *auth, uint32_t id, void *data) +{ + lrb_assert(id < rb_dlink_list_length(&auth_providers)); + auth->data[id].data = data; +} + +/* Set timeout relative to current time on provider + * When the timeout lapses, the provider's timeout call will execute */ +static inline void +set_provider_timeout_relative(struct auth_client *auth, uint32_t id, time_t timeout) +{ + lrb_assert(id < rb_dlink_list_length(&auth_providers)); + auth->data[id].timeout = timeout + rb_current_time(); +} + +/* Set timeout value in absolute time (Unix timestamp) + * When the timeout lapses, the provider's timeout call will execute */ +static inline void +set_provider_timeout_absolute(struct auth_client *auth, uint32_t id, time_t timeout) +{ + lrb_assert(id < rb_dlink_list_length(&auth_providers)); + auth->data[id].timeout = timeout; +} + +/* Get the timeout value for the provider */ +static inline time_t +get_provider_timeout(struct auth_client *auth, uint32_t id) +{ + lrb_assert(id < rb_dlink_list_length(&auth_providers)); + return auth->data[id].timeout; } #endif /* __CHARYBDIS_AUTHD_PROVIDER_H__ */ diff --git a/authd/providers/blacklist.c b/authd/providers/blacklist.c index d387541e..1c6abf45 100644 --- a/authd/providers/blacklist.c +++ b/authd/providers/blacklist.c @@ -356,7 +356,7 @@ blacklists_start(struct auth_client *auth) /* This probably can't happen but let's handle this case anyway */ lookup_all_blacklists(auth); - set_provider_on(auth, PROVIDER_BLACKLIST); + set_provider_running(auth, PROVIDER_BLACKLIST); return true; } diff --git a/authd/providers/ident.c b/authd/providers/ident.c index 9a31b911..2f08c550 100644 --- a/authd/providers/ident.c +++ b/authd/providers/ident.c @@ -324,7 +324,7 @@ ident_start(struct auth_client *auth) GET_SS_LEN(&l_addr), ident_connected, auth, ident_timeout); - set_provider_on(auth, PROVIDER_IDENT); + set_provider_running(auth, PROVIDER_IDENT); return true; } diff --git a/authd/providers/opm.c b/authd/providers/opm.c index 1d76f647..4f0b3d5c 100644 --- a/authd/providers/opm.c +++ b/authd/providers/opm.c @@ -669,7 +669,7 @@ opm_start(struct auth_client *auth) /* This probably can't happen but let's handle this case anyway */ opm_scan(auth); - set_provider_on(auth, PROVIDER_BLACKLIST); + set_provider_running(auth, PROVIDER_BLACKLIST); return true; } diff --git a/authd/providers/rdns.c b/authd/providers/rdns.c index 8b4dd500..25cd2813 100644 --- a/authd/providers/rdns.c +++ b/authd/providers/rdns.c @@ -131,7 +131,7 @@ rdns_start(struct auth_client *auth) query->query = lookup_hostname(auth->c_ip, dns_answer_callback, auth); notice_client(auth->cid, messages[REPORT_LOOKUP]); - set_provider_on(auth, PROVIDER_RDNS); + set_provider_running(auth, PROVIDER_RDNS); return true; }