diff mbox series

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

Message ID 20210218003956.23887-1-inga.stotland@intel.com
State New
Headers show
Series [BlueZ,v2] mesh: Combine common functions for NetKeys and AppKeys | expand

Commit Message

Inga Stotland Feb. 18, 2021, 12:39 a.m. UTC 
No change in functionality, code tightening.
---
 mesh/keyring.c | 117 +++++++++++++++++++++----------------------------
 1 file changed, 49 insertions(+), 68 deletions(-)

Comments

bluez.test.bot@gmail.com Feb. 18, 2021, 1:09 a.m. UTC | #1 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=434773

---Test result---

##############################
Test: CheckPatch - FAIL
Output:
mesh: Combine common functions for NetKeys and AppKeys
WARNING:SYMBOLIC_PERMS: Symbolic permissions 'S_IRUSR | S_IWUSR' are not preferred. Consider using octal permissions '0600'.
#49: FILE: mesh/keyring.c:58:
+		return open(fname, flags, S_IRUSR | S_IWUSR);

- total: 0 errors, 1 warnings, 174 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] mesh: Combine common functions for NetKeys and AppKeys" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth
Brian Gix Feb. 18, 2021, 7:05 p.m. UTC | #2 
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,
diff mbox series

Patch

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,