linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH BlueZ] mesh: Clean up config model publication set
@ 2020-06-26 21:59 Inga Stotland
  2020-06-26 22:18 ` [BlueZ] " bluez.test.bot
  2020-06-30 17:20 ` [PATCH BlueZ] " Gix, Brian
  0 siblings, 2 replies; 3+ messages in thread
From: Inga Stotland @ 2020-06-26 21:59 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: brian.gix, Inga Stotland

No functioal changes. Make code more readable, plus style fixes.
---
 mesh/cfgmod-server.c | 77 +++++++++++++++++++++-----------------------
 1 file changed, 37 insertions(+), 40 deletions(-)

diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
index 902228ae0..c525d9d24 100644
--- a/mesh/cfgmod-server.c
+++ b/mesh/cfgmod-server.c
@@ -46,12 +46,10 @@ static void send_pub_status(struct mesh_node *node, uint16_t net_idx,
 	n = mesh_model_opcode_set(OP_CONFIG_MODEL_PUB_STATUS, msg);
 	msg[n++] = status;
 	l_put_le16(ele_addr, msg + n);
-	n += 2;
-	l_put_le16(pub_addr, msg + n);
-	n += 2;
+	l_put_le16(pub_addr, msg + n + 2);
 	idx |= cred_flag ? CREDFLAG_MASK : 0;
-	l_put_le16(idx, msg + n);
-	n += 2;
+	l_put_le16(idx, msg + n + 4);
+	n += 6;
 	msg[n++] = ttl;
 	msg[n++] = period;
 	msg[n++] = retransmit;
@@ -61,16 +59,15 @@ static void send_pub_status(struct mesh_node *node, uint16_t net_idx,
 		n += 2;
 	} else {
 		l_put_le16(mod_id >> 16, msg + n);
-		n += 2;
-		l_put_le16(mod_id, msg + n);
-		n += 2;
+		l_put_le16(mod_id, msg + n + 2);
+		n += 4;
 	}
 
 	mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, net_idx, DEFAULT_TTL,
 								false, msg, n);
 }
 
-static bool config_pub_get(struct mesh_node *node, uint16_t net_idx,
+static void config_pub_get(struct mesh_node *node, uint16_t net_idx,
 					uint16_t src, uint16_t dst,
 					const uint8_t *pkt, uint16_t size)
 {
@@ -86,7 +83,7 @@ static bool config_pub_get(struct mesh_node *node, uint16_t net_idx,
 		mod_id = l_get_le16(pkt + 2) << 16;
 		mod_id |= l_get_le16(pkt + 4);
 	} else
-		return false;
+		return;
 
 	ele_addr = l_get_le16(pkt);
 	pub = mesh_model_pub_get(node, ele_addr, mod_id, &status);
@@ -98,16 +95,15 @@ static bool config_pub_get(struct mesh_node *node, uint16_t net_idx,
 	else
 		send_pub_status(node, net_idx, src, dst, status, ele_addr,
 				mod_id, 0, 0, 0, 0, 0, 0);
-	return true;
 }
 
 static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
 					uint16_t src, uint16_t dst,
-					const uint8_t *pkt, uint8_t virt_offset,
+					const uint8_t *pkt, bool virt,
 					bool vendor, bool unreliable)
 {
 	uint32_t mod_id;
-	uint16_t ele_addr, idx, ota = 0;
+	uint16_t ele_addr, idx, ota = UNASSIGNED_ADDRESS;
 	const uint8_t *pub_addr;
 	uint16_t test_addr;
 	uint8_t ttl, period;
@@ -115,34 +111,33 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
 	int status;
 	bool cred_flag;
 
-	idx = l_get_le16(pkt + 4 + virt_offset);
-	ttl = pkt[6 + virt_offset];
-	period = pkt[7 + virt_offset];
-	retransmit = pkt[8 + virt_offset];
-	mod_id = l_get_le16(pkt + 9 + virt_offset);
+	ele_addr = l_get_le16(pkt);
+	pub_addr = pkt + 2;
+
+	pkt += (virt ? 14 : 0);
+
+	idx = l_get_le16(pkt + 4);
+	ttl = pkt[6];
+	period = pkt[7];
+	retransmit = pkt[8];
+	mod_id = l_get_le16(pkt + 9);
 
 	if (!vendor)
 		mod_id |= VENDOR_ID_MASK;
 	else
-		mod_id = (mod_id << 16) | l_get_le16(pkt + 11 + virt_offset);
-
-	ele_addr = l_get_le16(pkt);
-	pub_addr = pkt + 2;
+		mod_id = (mod_id << 16) | l_get_le16(pkt + 11);
 
-	/* Doesn't accept virtual seeming addresses */
+	/* Don't accept virtual seeming addresses */
 	test_addr = l_get_le16(pub_addr);
-	if (!virt_offset && IS_VIRTUAL(test_addr))
+	if (!virt && IS_VIRTUAL(test_addr))
 		return;
 
-	/* Get cred_flag */
 	cred_flag = !!(CREDFLAG_MASK & idx);
-
-	/* Get AppKey index */
 	idx &= APP_IDX_MASK;
 
 	status = mesh_model_pub_set(node, ele_addr, mod_id, pub_addr, idx,
 					cred_flag, ttl, period, retransmit,
-					virt_offset != 0, &ota);
+					virt, &ota);
 
 	l_debug("pub_set: status %d, ea %4.4x, ota: %4.4x, mod: %x, idx: %3.3x",
 					status, ele_addr, ota, mod_id, idx);
@@ -155,16 +150,17 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
 		return;
 	}
 
-	if (IS_UNASSIGNED(ota) && !virt_offset) {
+	if (IS_UNASSIGNED(test_addr) && !virt) {
 		ttl = period = idx = 0;
 
 		/* Remove model publication from config file */
-		mesh_config_model_pub_del(node_config_get(node), ele_addr,
+		if (!mesh_config_model_pub_del(node_config_get(node), ele_addr,
 				vendor ? mod_id : mod_id & ~VENDOR_ID_MASK,
-								vendor);
+									vendor))
+			status = MESH_STATUS_STORAGE_FAIL;
 	} else {
 		struct mesh_config_pub db_pub = {
-			.virt = (virt_offset != 0),
+			.virt = virt,
 			.addr = ota,
 			.idx = idx,
 			.ttl = ttl,
@@ -174,7 +170,7 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
 			.interval = ((retransmit >> 3) + 1) * 50
 		};
 
-		if (virt_offset)
+		if (virt)
 			memcpy(db_pub.virt_addr, pub_addr, 16);
 
 		/* Save model publication to config file */
@@ -186,7 +182,8 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
 
 	if (!unreliable)
 		send_pub_status(node, net_idx, src, dst, status, ele_addr,
-			mod_id, ota, idx, cred_flag, ttl, period, retransmit);
+					mod_id, ota, idx, cred_flag, ttl,
+					period, retransmit);
 }
 
 static void send_sub_status(struct mesh_node *node, uint16_t net_idx,
@@ -781,16 +778,16 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
 		if (size != 25 && size != 27)
 			return true;
 
-		config_pub_set(node, net_idx, src, dst, pkt, 14, size == 27,
-				!!(opcode & OP_UNRELIABLE));
-		break;
+		virt = true;
+		/* Fall Through */
 
 	case OP_CONFIG_MODEL_PUB_SET:
-		if (size != 11 && size != 13)
+		if (!virt && (size != 11 && size != 13))
 			return true;
 
-		config_pub_set(node, net_idx, src, dst, pkt, 0, size == 13,
-				!!(opcode & OP_UNRELIABLE));
+		config_pub_set(node, net_idx, src, dst, pkt, virt,
+						size == 13 || size == 27,
+						!!(opcode & OP_UNRELIABLE));
 		break;
 
 	case OP_CONFIG_MODEL_PUB_GET:
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* RE: [BlueZ] mesh: Clean up config model publication set
  2020-06-26 21:59 [PATCH BlueZ] mesh: Clean up config model publication set Inga Stotland
@ 2020-06-26 22:18 ` bluez.test.bot
  2020-06-30 17:20 ` [PATCH BlueZ] " Gix, Brian
  1 sibling, 0 replies; 3+ messages in thread
From: bluez.test.bot @ 2020-06-26 22:18 UTC (permalink / raw)
  To: linux-bluetooth, inga.stotland

[-- Attachment #1: Type: text/plain, Size: 970 bytes --]


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.
While we are preparing for reviewing the patches, we found the following
issue/warning.

Test Result:
checkpatch Failed

Outputs:
WARNING:PREFER_FALLTHROUGH: Prefer 'fallthrough;' over fallthrough comment
#174: FILE: mesh/cfgmod-server.c:782:
+		/* Fall Through */

- total: 0 errors, 1 warnings, 168 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.

Your patch 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.



---
Regards,
Linux Bluetooth

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH BlueZ] mesh: Clean up config model publication set
  2020-06-26 21:59 [PATCH BlueZ] mesh: Clean up config model publication set Inga Stotland
  2020-06-26 22:18 ` [BlueZ] " bluez.test.bot
@ 2020-06-30 17:20 ` Gix, Brian
  1 sibling, 0 replies; 3+ messages in thread
From: Gix, Brian @ 2020-06-30 17:20 UTC (permalink / raw)
  To: linux-bluetooth, Stotland, Inga

Applied

On Fri, 2020-06-26 at 14:59 -0700, Inga Stotland wrote:
> No functioal changes. Make code more readable, plus style fixes.
> ---
>  mesh/cfgmod-server.c | 77 +++++++++++++++++++++-----------------------
>  1 file changed, 37 insertions(+), 40 deletions(-)
> 
> diff --git a/mesh/cfgmod-server.c b/mesh/cfgmod-server.c
> index 902228ae0..c525d9d24 100644
> --- a/mesh/cfgmod-server.c
> +++ b/mesh/cfgmod-server.c
> @@ -46,12 +46,10 @@ static void send_pub_status(struct mesh_node *node, uint16_t net_idx,
>  	n = mesh_model_opcode_set(OP_CONFIG_MODEL_PUB_STATUS, msg);
>  	msg[n++] = status;
>  	l_put_le16(ele_addr, msg + n);
> -	n += 2;
> -	l_put_le16(pub_addr, msg + n);
> -	n += 2;
> +	l_put_le16(pub_addr, msg + n + 2);
>  	idx |= cred_flag ? CREDFLAG_MASK : 0;
> -	l_put_le16(idx, msg + n);
> -	n += 2;
> +	l_put_le16(idx, msg + n + 4);
> +	n += 6;
>  	msg[n++] = ttl;
>  	msg[n++] = period;
>  	msg[n++] = retransmit;
> @@ -61,16 +59,15 @@ static void send_pub_status(struct mesh_node *node, uint16_t net_idx,
>  		n += 2;
>  	} else {
>  		l_put_le16(mod_id >> 16, msg + n);
> -		n += 2;
> -		l_put_le16(mod_id, msg + n);
> -		n += 2;
> +		l_put_le16(mod_id, msg + n + 2);
> +		n += 4;
>  	}
>  
>  	mesh_model_send(node, dst, src, APP_IDX_DEV_LOCAL, net_idx, DEFAULT_TTL,
>  								false, msg, n);
>  }
>  
> -static bool config_pub_get(struct mesh_node *node, uint16_t net_idx,
> +static void config_pub_get(struct mesh_node *node, uint16_t net_idx,
>  					uint16_t src, uint16_t dst,
>  					const uint8_t *pkt, uint16_t size)
>  {
> @@ -86,7 +83,7 @@ static bool config_pub_get(struct mesh_node *node, uint16_t net_idx,
>  		mod_id = l_get_le16(pkt + 2) << 16;
>  		mod_id |= l_get_le16(pkt + 4);
>  	} else
> -		return false;
> +		return;
>  
>  	ele_addr = l_get_le16(pkt);
>  	pub = mesh_model_pub_get(node, ele_addr, mod_id, &status);
> @@ -98,16 +95,15 @@ static bool config_pub_get(struct mesh_node *node, uint16_t net_idx,
>  	else
>  		send_pub_status(node, net_idx, src, dst, status, ele_addr,
>  				mod_id, 0, 0, 0, 0, 0, 0);
> -	return true;
>  }
>  
>  static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
>  					uint16_t src, uint16_t dst,
> -					const uint8_t *pkt, uint8_t virt_offset,
> +					const uint8_t *pkt, bool virt,
>  					bool vendor, bool unreliable)
>  {
>  	uint32_t mod_id;
> -	uint16_t ele_addr, idx, ota = 0;
> +	uint16_t ele_addr, idx, ota = UNASSIGNED_ADDRESS;
>  	const uint8_t *pub_addr;
>  	uint16_t test_addr;
>  	uint8_t ttl, period;
> @@ -115,34 +111,33 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
>  	int status;
>  	bool cred_flag;
>  
> -	idx = l_get_le16(pkt + 4 + virt_offset);
> -	ttl = pkt[6 + virt_offset];
> -	period = pkt[7 + virt_offset];
> -	retransmit = pkt[8 + virt_offset];
> -	mod_id = l_get_le16(pkt + 9 + virt_offset);
> +	ele_addr = l_get_le16(pkt);
> +	pub_addr = pkt + 2;
> +
> +	pkt += (virt ? 14 : 0);
> +
> +	idx = l_get_le16(pkt + 4);
> +	ttl = pkt[6];
> +	period = pkt[7];
> +	retransmit = pkt[8];
> +	mod_id = l_get_le16(pkt + 9);
>  
>  	if (!vendor)
>  		mod_id |= VENDOR_ID_MASK;
>  	else
> -		mod_id = (mod_id << 16) | l_get_le16(pkt + 11 + virt_offset);
> -
> -	ele_addr = l_get_le16(pkt);
> -	pub_addr = pkt + 2;
> +		mod_id = (mod_id << 16) | l_get_le16(pkt + 11);
>  
> -	/* Doesn't accept virtual seeming addresses */
> +	/* Don't accept virtual seeming addresses */
>  	test_addr = l_get_le16(pub_addr);
> -	if (!virt_offset && IS_VIRTUAL(test_addr))
> +	if (!virt && IS_VIRTUAL(test_addr))
>  		return;
>  
> -	/* Get cred_flag */
>  	cred_flag = !!(CREDFLAG_MASK & idx);
> -
> -	/* Get AppKey index */
>  	idx &= APP_IDX_MASK;
>  
>  	status = mesh_model_pub_set(node, ele_addr, mod_id, pub_addr, idx,
>  					cred_flag, ttl, period, retransmit,
> -					virt_offset != 0, &ota);
> +					virt, &ota);
>  
>  	l_debug("pub_set: status %d, ea %4.4x, ota: %4.4x, mod: %x, idx: %3.3x",
>  					status, ele_addr, ota, mod_id, idx);
> @@ -155,16 +150,17 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
>  		return;
>  	}
>  
> -	if (IS_UNASSIGNED(ota) && !virt_offset) {
> +	if (IS_UNASSIGNED(test_addr) && !virt) {
>  		ttl = period = idx = 0;
>  
>  		/* Remove model publication from config file */
> -		mesh_config_model_pub_del(node_config_get(node), ele_addr,
> +		if (!mesh_config_model_pub_del(node_config_get(node), ele_addr,
>  				vendor ? mod_id : mod_id & ~VENDOR_ID_MASK,
> -								vendor);
> +									vendor))
> +			status = MESH_STATUS_STORAGE_FAIL;
>  	} else {
>  		struct mesh_config_pub db_pub = {
> -			.virt = (virt_offset != 0),
> +			.virt = virt,
>  			.addr = ota,
>  			.idx = idx,
>  			.ttl = ttl,
> @@ -174,7 +170,7 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
>  			.interval = ((retransmit >> 3) + 1) * 50
>  		};
>  
> -		if (virt_offset)
> +		if (virt)
>  			memcpy(db_pub.virt_addr, pub_addr, 16);
>  
>  		/* Save model publication to config file */
> @@ -186,7 +182,8 @@ static void config_pub_set(struct mesh_node *node, uint16_t net_idx,
>  
>  	if (!unreliable)
>  		send_pub_status(node, net_idx, src, dst, status, ele_addr,
> -			mod_id, ota, idx, cred_flag, ttl, period, retransmit);
> +					mod_id, ota, idx, cred_flag, ttl,
> +					period, retransmit);
>  }
>  
>  static void send_sub_status(struct mesh_node *node, uint16_t net_idx,
> @@ -781,16 +778,16 @@ static bool cfg_srv_pkt(uint16_t src, uint16_t dst, uint16_t app_idx,
>  		if (size != 25 && size != 27)
>  			return true;
>  
> -		config_pub_set(node, net_idx, src, dst, pkt, 14, size == 27,
> -				!!(opcode & OP_UNRELIABLE));
> -		break;
> +		virt = true;
> +		/* Fall Through */
>  
>  	case OP_CONFIG_MODEL_PUB_SET:
> -		if (size != 11 && size != 13)
> +		if (!virt && (size != 11 && size != 13))
>  			return true;
>  
> -		config_pub_set(node, net_idx, src, dst, pkt, 0, size == 13,
> -				!!(opcode & OP_UNRELIABLE));
> +		config_pub_set(node, net_idx, src, dst, pkt, virt,
> +						size == 13 || size == 27,
> +						!!(opcode & OP_UNRELIABLE));
>  		break;
>  
>  	case OP_CONFIG_MODEL_PUB_GET:

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-06-30 17:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-26 21:59 [PATCH BlueZ] mesh: Clean up config model publication set Inga Stotland
2020-06-26 22:18 ` [BlueZ] " bluez.test.bot
2020-06-30 17:20 ` [PATCH BlueZ] " Gix, Brian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).