2014-10-28 15:20:01

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

---
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

2014-10-31 10:34:56

by Andrei Emeltchenko

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

ping

On Tue, Oct 28, 2014 at 05:20:01PM +0200, Andrei Emeltchenko wrote:
> From: Andrei Emeltchenko <[email protected]>
>
> ---
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2014-10-28 15:20:02

by Andrei Emeltchenko

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

From: Andrei Emeltchenko <[email protected]>

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

2014-11-03 20:20:20

by Johan Hedberg

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

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

2014-11-03 20:18:25

by Johan Hedberg

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

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