[PATCHv2 1/2] gatt: Fix wrong function exit

[PATCHv2 1/2] gatt: Fix wrong function exit

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

---
 src/shared/gatt-helpers.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index d751d5a..4f53de3 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -1029,7 +1029,7 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,
 		data = new_read_included(cur_result);
 		if (!data) {
 			success = false;
-			goto done;
+			goto failed;
 		}
 
 		read_included(data);
-- 
1.9.1

[PATCHv2 2/2] gatt: Refactor discover_included_cb()

[Andrei Emeltchenko]

From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>

Initialize variable at declaration fixing setting it to the same value
many times.
---
 src/shared/gatt-helpers.c | 20 +++++---------------
 1 file changed, 5 insertions(+), 15 deletions(-)

diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
index 4f53de3..5713924 100644
--- a/src/shared/gatt-helpers.c
+++ b/src/shared/gatt-helpers.c
@@ -974,7 +974,7 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,
 	uint8_t att_ecode = 0;
 	uint16_t last_handle;
 	size_t data_length;
-	bool success;
+	bool success = false;
 
 	if (opcode == BT_ATT_OP_ERROR_RSP) {
 		att_ecode = process_error(pdu, length);
@@ -983,14 +983,11 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,
 							op->result_head)
 			goto done;
 
-		success = false;
 		goto failed;
 	}
 
-	if (opcode != BT_ATT_OP_READ_BY_TYPE_RSP || !pdu || length < 6) {
-		success = false;
+	if (opcode != BT_ATT_OP_READ_BY_TYPE_RSP || !pdu || length < 6)
 		goto failed;
-	}
 
 	data_length = ((const uint8_t *) pdu)[0];
 
@@ -1004,17 +1001,13 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,
 	 * optional 2 octets - Bluetooth UUID of included service
 	 */
 	if ((data_length != 8 && data_length != 6) ||
-						(length - 1) % data_length) {
-		success = false;
+						(length - 1) % data_length)
 		goto failed;
-	}
 
 	cur_result = result_create(opcode, pdu + 1, length - 1,
 							data_length, op);
-	if (!cur_result) {
-		success = false;
+	if (!cur_result)
 		goto failed;
-	}
 
 	if (!op->result_head) {
 		op->result_head = op->result_tail = cur_result;
@@ -1027,10 +1020,8 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,
 		struct read_incl_data *data;
 
 		data = new_read_included(cur_result);
-		if (!data) {
-			success = false;
+		if (!data)
 			goto failed;
-		}
 
 		read_included(data);
 		return;
@@ -1052,7 +1043,6 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,
 			return;
 
 		discovery_op_unref(op);
-		success = false;
 		goto failed;
 	}
 
-- 
1.9.1

Re: [PATCHv2 1/2] gatt: Fix wrong function exit

[Andrei Emeltchenko]

ping

On Tue, Oct 28, 2014 at 05:20:01PM +0200, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <andrei.emeltchenko@intel.com>
> 
> ---
>  src/shared/gatt-helpers.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index d751d5a..4f53de3 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -1029,7 +1029,7 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,
>  		data = new_read_included(cur_result);
>  		if (!data) {
>  			success = false;
> -			goto done;
> +			goto failed;
>  		}
>  
>  		read_included(data);
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 1/2] gatt: Fix wrong function exit

[Johan Hedberg]

Hi Andrei,

On Tue, Oct 28, 2014, Andrei Emeltchenko wrote:
> ---
>  src/shared/gatt-helpers.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This patch has been applied. Thanks.

Johan

Re: [PATCHv2 2/2] gatt: Refactor discover_included_cb()

[Johan Hedberg]

Hi Andrei,

On Tue, Oct 28, 2014, Andrei Emeltchenko wrote:
> Initialize variable at declaration fixing setting it to the same value
> many times.
> ---
>  src/shared/gatt-helpers.c | 20 +++++---------------
>  1 file changed, 5 insertions(+), 15 deletions(-)
> 
> diff --git a/src/shared/gatt-helpers.c b/src/shared/gatt-helpers.c
> index 4f53de3..5713924 100644
> --- a/src/shared/gatt-helpers.c
> +++ b/src/shared/gatt-helpers.c
> @@ -974,7 +974,7 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,
>  	uint8_t att_ecode = 0;
>  	uint16_t last_handle;
>  	size_t data_length;
> -	bool success;
> +	bool success = false;
>  
>  	if (opcode == BT_ATT_OP_ERROR_RSP) {
>  		att_ecode = process_error(pdu, length);
> @@ -983,14 +983,11 @@ static void discover_included_cb(uint8_t opcode, const void *pdu,
>  							op->result_head)
>  			goto done;
>  
> -		success = false;
>  		goto failed;
>  	}

We generally try to avoid variable initializations upon declaration to
keep it clear what value is expected and where as well as make it easy
to spot unused variables. So I'm not really convinced of this patch.

Johan