Patch: Add init and cleanup functions to registry backends and call them if they are defined

[Jeremy Allison]

On Sun, Jan 11, 2015 at 11:59:44AM -0800, Richard Sharpe wrote:
> On Sat, Jan 10, 2015 at 1:02 PM, Richard Sharpe
> <realrichardsharpe at gmail.com> wrote:
> > Hi folks,
> >
> > Sometimes, as in my zookeeper backend, you need an init and cleanup function.
> >
> > This small patch provides that.
> >
> > Please provide feedback or push if OK ...
> 
> A small update. If the init function fails, we should not install it.
> Rather, we should print a level 0 message in the log.

Can you call the functions registry_ops_init()/registry_ops_cleanup()
not just init() and cleanup() please ?

Also, rather than calling these inside the reghook_cache_add()
function, shouldn't these be called inside reghook_cache_init() ?


> From 9de02e3617650b6295e584f4ea55512fd12ec68e Mon Sep 17 00:00:00 2001
> From: Richard Sharpe <realrichardsharpe at gmail.com>
> Date: Sat, 10 Jan 2015 12:58:37 -0800
> Subject: [PATCH] Add init and cleanup functions to the registry backends and
>  call them when them when a registry backend is installed, if they are
>  defined. If the init function for a new backend fails, we will refuse to
>  install it to prevent further damage and we print a log level 0 error
>  message.
> 
> Signed-off-by: Richard Sharpe <rsharpe at samba.org>
> ---
>  source3/include/registry.h       |  3 +++
>  source3/registry/reg_cachehook.c | 24 ++++++++++++++++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/source3/include/registry.h b/source3/include/registry.h
> index f7a537e..008046a 100644
> --- a/source3/include/registry.h
> +++ b/source3/include/registry.h
> @@ -40,6 +40,9 @@ struct regsubkey_ctr;
>   */
>  
>  struct registry_ops {
> +	/* Initialize and cleanup the back end, if needed ... */
> +	int	(*init)(const char *base_key);
> +	int	(*cleanup)(void);
>  	/* functions for enumerating subkeys and values */
>  	int 	(*fetch_subkeys)( const char *key, struct regsubkey_ctr *subkeys);
>  	int 	(*fetch_values) ( const char *key, struct regval_ctr *val );
> diff --git a/source3/registry/reg_cachehook.c b/source3/registry/reg_cachehook.c
> index 1f26927..ae53096 100644
> --- a/source3/registry/reg_cachehook.c
> +++ b/source3/registry/reg_cachehook.c
> @@ -80,6 +80,7 @@ WERROR reghook_cache_add(const char *keyname, struct registry_ops *ops)
>  {
>  	WERROR werr;
>  	char *key = NULL;
> +	struct registry_ops *old_ops;
>  
>  	if ((keyname == NULL) || (ops == NULL)) {
>  		return WERR_INVALID_PARAM;
> @@ -93,6 +94,29 @@ WERROR reghook_cache_add(const char *keyname, struct registry_ops *ops)
>  	DEBUG(10, ("reghook_cache_add: Adding ops %p for key [%s]\n",
>  		   (void *)ops, key));
>  
> +	/*
> +	 * If there is already an ops structure registered, call its 
> +	 * cleanup function, if any.
> +	 */
> +	if ((old_ops = pathtree_find(cache_tree, key)) && old_ops->cleanup) {
> +		DEBUG(10, ("Calling cleanup function for old ops: %p\n",
> +			old_ops));
> +		old_ops->cleanup();
> +	}
> +
> +	/*
> +	 * Call the init function, if it exists ... but if the init
> +	 * function fails, do not install the new ops.
> +	 */
> +	if (ops->init) {
> +		if (!ops->init(key)) {
> +			DEBUG(0, ("ERROR: Init function failed for ops:key %p:%s, not installed\n",
> +				ops, key));
> +			werr = WERR_NOMEM; /* Is there something better? */
> +			goto done; 
> +		}
> +	}
> +
>  	if (!pathtree_add(cache_tree, key, ops))
>  		werr = WERR_NOMEM;
>  	else
> -- 
> 1.8.3.1
>