[PATCH] Add remote arch caching

[Volker Lendecke]

Hi, Ralph!

On Mon, Apr 25, 2016 at 03:13:50PM +0200, Ralph Boehme wrote:
> -static const char *remote_arch_str;
> +static const char *remote_arch_strings[] = {
> +	[RA_UNKNOWN] =	"UNKNOWN",
> +	[RA_WFWG] =	"WfWg",

... cool stuff :-)

>  const char *get_remote_arch_str(void)
>  {
> -	if (!remote_arch_str) {
> -		return "UNKNOWN";
> -	}
> -	return remote_arch_str;
> +	return remote_arch_strings[ra_type];
>  }

Question -- do we want range checking here? I don't think compilers
enforce the enum ranges.

> +enum remote_arch_types get_remote_arch_from_str(const char *remote_arch_string)
> +{
> +	for (int i = 0; i < sizeof(remote_arch_strings); i++) {

Shouldn't that be ARRAY_SIZE(remote_arch_strings)?

> +		if (strcmp(remote_arch_string, remote_arch_strings[i]) == 0) {
> +			return i;
> +		}
> +	}
> +	return RA_UNKNOWN;
> +}
> +
>  /*******************************************************************
>   Set the horrid remote_arch string based on an enum.
>  ********************************************************************/
> -- 
> 2.5.0
> 
> 
> From 24e2e873bd24886a0bf316deabfa4442c1e34eae Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 13 Apr 2016 17:42:55 +0200
> Subject: [PATCH 3/4] s3/lib: add remote arch caching
> 
> This allows caching the remote arch string in gencache. A subsequent
> commit will use this in SMB2 negprot.
> 
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
>  source3/include/proto.h |   4 ++
>  source3/lib/util.c      | 105 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 109 insertions(+)
> 
> diff --git a/source3/include/proto.h b/source3/include/proto.h
> index 8baf9d9..05efd2e 100644
> --- a/source3/include/proto.h
> +++ b/source3/include/proto.h
> @@ -392,6 +392,10 @@ const char *get_remote_arch_str(void);
>  enum remote_arch_types get_remote_arch_from_str(const char *remote_arch_string);
>  void set_remote_arch(enum remote_arch_types type);
>  enum remote_arch_types get_remote_arch(void);
> +bool remote_arch_cache_update(TALLOC_CTX *mem_ctx,
> +			      const struct GUID *client_guid);
> +bool remote_arch_cache_delete(TALLOC_CTX *mem_ctx,
> +			      const struct GUID *client_guid);
>  const char *tab_depth(int level, int depth);
>  int str_checksum(const char *s);
>  void zero_free(void *p, size_t size);
> diff --git a/source3/lib/util.c b/source3/lib/util.c
> index 53a6d79..9fe8948 100644
> --- a/source3/lib/util.c
> +++ b/source3/lib/util.c
> @@ -1292,6 +1292,111 @@ enum remote_arch_types get_remote_arch(void)
>  	return ra_type;
>  }
>  
> +#define RA_CACHE_TTL 7*24*3600
> +
> +static char *remote_arch_cache_key(TALLOC_CTX *mem_ctx,
> +				   const struct GUID *client_guid)
> +{
> +	char *guid_string = NULL;
> +	char *key = NULL;
> +
> +	guid_string = GUID_string(mem_ctx, client_guid);
> +	if (guid_string == NULL) {
> +		return NULL;
> +	}

Probably it does not matter significantly performance-wise, but we
have GUID_buf_string(). Can we use that here?

> +static bool remote_arch_cache_get(TALLOC_CTX *mem_ctx,
> +				  const struct GUID *client_guid)
> +{
> +	bool found;
> +	char *ra_key = NULL;
> +	char *ra_str = NULL;
> +	enum remote_arch_types ra;
> +
> +	ra_key = remote_arch_cache_key(mem_ctx, client_guid);
> +	if (ra_key == NULL) {
> +		return false;
> +	}
> +
> +	found = gencache_get(ra_key, mem_ctx, &ra_str, NULL);
> +	TALLOC_FREE(ra_key);
> +	if (!found) {
> +		return true;
> +	}
> +
> +	ra = get_remote_arch_from_str(ra_str);
> +	if (ra != RA_UNKNOWN) {
> +		DBG_INFO("Settting remote arch [%s] from cache\n", ra_str);
> +		set_remote_arch(ra);
> +	}
> +
> +	return true;
> +}

Doesn't this leak "ra_str"? gencache_parse() would avoid that....

Volker

-- 
SerNet GmbH, Bahnhofsallee 1b, 37081 Göttingen
phone: +49-551-370000-0, fax: +49-551-370000-9
AG Göttingen, HRB 2816, GF: Dr. Johannes Loxen
http://www.sernet.de, mailto:kontakt at sernet.de