Re: [PATCH BlueZ v2] mesh: Combine common functions for NetKeys and AppKeys

["Gix, Brian" <brian.gix@intel.com>]

Applied with additional tweek of changing file modes from symbolic names to 0600 to conform to new tests in
checkpatch and kernel standards.

On Wed, 2021-02-17 at 16:39 -0800, Inga Stotland wrote:
> No change in functionality, code tightening.
> ---
>  mesh/keyring.c | 117 +++++++++++++++++++++----------------------------
>  1 file changed, 49 insertions(+), 68 deletions(-)
> 
> diff --git a/mesh/keyring.c b/mesh/keyring.c
> index 0b74ee914..cb04437ed 100644
> --- a/mesh/keyring.c
> +++ b/mesh/keyring.c
> @@ -33,31 +33,45 @@ const char *dev_key_dir = "/dev_keys";
>  const char *app_key_dir = "/app_keys";
>  const char *net_key_dir = "/net_keys";
>  
> -bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,
> -						struct keyring_net_key *key)
> +static int open_key_file(struct mesh_node *node, const char *key_dir,
> +							uint16_t idx, int flags)
>  {
>  	const char *node_path;
> -	char key_file[PATH_MAX];
> -	bool result = false;
> -	int fd;
> +	char fname[PATH_MAX];
>  
> -	if (!node || !key)
> -		return false;
> +	if (!node)
> +		return -1;
>  
>  	node_path = node_get_storage_dir(node);
>  
> -	if (strlen(node_path) + strlen(net_key_dir) + 1 + 3 >= PATH_MAX)
> -		return false;
> +	if (strlen(node_path) + strlen(key_dir) + 1 + 3 >= PATH_MAX)
> +		return -1;
> +
> +	if (flags & O_CREAT) {
> +		snprintf(fname, PATH_MAX, "%s%s", node_path, key_dir);
> +		mkdir(fname, 0755);
> +	}
>  
> -	snprintf(key_file, PATH_MAX, "%s%s", node_path, net_key_dir);
> +	snprintf(fname, PATH_MAX, "%s%s/%3.3x", node_path, key_dir, idx);
>  
> -	mkdir(key_file, 0755);
> +	if (flags & O_CREAT)
> +		return open(fname, flags, S_IRUSR | S_IWUSR);
> +	else
> +		return open(fname, flags);
> +}
>  
> -	snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, net_key_dir,
> -								net_idx);
> -	l_debug("Put Net Key %s", key_file);
> +bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,
> +						struct keyring_net_key *key)
> +{
> +	bool result = false;
> +	int fd;
> +
> +	if (!key)
> +		return false;
> +
> +	fd = open_key_file(node, net_key_dir, net_idx,
> +					O_WRONLY | O_CREAT | O_TRUNC);
>  
> -	fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR);
>  	if (fd < 0)
>  		return false;
>  
> @@ -72,28 +86,14 @@ bool keyring_put_net_key(struct mesh_node *node, uint16_t net_idx,
>  bool keyring_put_app_key(struct mesh_node *node, uint16_t app_idx,
>  				uint16_t net_idx, struct keyring_app_key *key)
>  {
> -	const char *node_path;
> -	char key_file[PATH_MAX];
>  	bool result = false;
>  	int fd;
>  
> -	if (!node || !key)
> -		return false;
> -
> -	node_path = node_get_storage_dir(node);
> -
> -	if (strlen(node_path) + strlen(app_key_dir) + 1 + 3 >= PATH_MAX)
> +	if (!key)
>  		return false;
>  
> -	snprintf(key_file, PATH_MAX, "%s%s", node_path, app_key_dir);
> -
> -	mkdir(key_file, 0755);
> +	fd = open_key_file(node, app_key_dir, app_idx, O_RDWR);
>  
> -	snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir,
> -								app_idx);
> -	l_debug("Put App Key %s", key_file);
> -
> -	fd = open(key_file, O_RDWR);
>  	if (fd >= 0) {
>  		struct keyring_app_key old_key;
>  
> @@ -105,12 +105,12 @@ bool keyring_put_app_key(struct mesh_node *node, uint16_t app_idx,
>  		}
>  
>  		lseek(fd, 0, SEEK_SET);
> -	} else {
> -		fd = open(key_file, O_WRONLY | O_CREAT | O_TRUNC,
> -							S_IRUSR | S_IWUSR);
> -		if (fd < 0)
> -			return false;
> -	}
> +	} else
> +		fd = open_key_file(node, app_key_dir, app_idx,
> +						O_WRONLY | O_CREAT | O_TRUNC);
> +
> +	if (fd < 0)
> +		return false;
>  
>  	if (write(fd, key, sizeof(*key)) == sizeof(*key))
>  		result = true;
> @@ -226,24 +226,19 @@ bool keyring_put_remote_dev_key(struct mesh_node *node, uint16_t unicast,
>  	return result;
>  }
>  
> -bool keyring_get_net_key(struct mesh_node *node, uint16_t net_idx,
> -						struct keyring_net_key *key)
> +static bool get_key(struct mesh_node *node, const char *key_dir,
> +					uint16_t key_idx, void *key, ssize_t sz)
>  {
> -	const char *node_path;
> -	char key_file[PATH_MAX];
>  	bool result = false;
>  	int fd;
>  
> -	if (!node || !key)
> +	if (!key)
>  		return false;
>  
> -	node_path = node_get_storage_dir(node);
> -	snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, net_key_dir,
> -								net_idx);
> +	fd = open_key_file(node, key_dir, key_idx, O_RDONLY);
>  
> -	fd = open(key_file, O_RDONLY);
>  	if (fd >= 0) {
> -		if (read(fd, key, sizeof(*key)) == sizeof(*key))
> +		if (read(fd, key, sz) == sz)
>  			result = true;
>  
>  		close(fd);
> @@ -252,30 +247,16 @@ bool keyring_get_net_key(struct mesh_node *node, uint16_t net_idx,
>  	return result;
>  }
>  
> +bool keyring_get_net_key(struct mesh_node *node, uint16_t net_idx,
> +						struct keyring_net_key *key)
> +{
> +	return get_key(node, net_key_dir, net_idx, key, sizeof(*key));
> +}
> +
>  bool keyring_get_app_key(struct mesh_node *node, uint16_t app_idx,
>  						struct keyring_app_key *key)
>  {
> -	const char *node_path;
> -	char key_file[PATH_MAX];
> -	bool result = false;
> -	int fd;
> -
> -	if (!node || !key)
> -		return false;
> -
> -	node_path = node_get_storage_dir(node);
> -	snprintf(key_file, PATH_MAX, "%s%s/%3.3x", node_path, app_key_dir,
> -								app_idx);
> -
> -	fd = open(key_file, O_RDONLY);
> -	if (fd >= 0) {
> -		if (read(fd, key, sizeof(*key)) == sizeof(*key))
> -			result = true;
> -
> -		close(fd);
> -	}
> -
> -	return result;
> +	return get_key(node, app_key_dir, app_idx, key, sizeof(*key));
>  }
>  
>  bool keyring_get_remote_dev_key(struct mesh_node *node, uint16_t unicast,