[PATCH 1/2] HDP: Fix checking always constant error code

[PATCH 1/2] HDP: Fix checking always constant error code

[Andrei Emeltchenko]

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

sdp_set_access_protos() always returns 0, there is no sense to check for
error code. Fixes compile warnings.
---
 profiles/health/hdp_util.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
index 58770b5..ff427a6 100644
--- a/profiles/health/hdp_util.c
+++ b/profiles/health/hdp_util.c
@@ -362,7 +362,7 @@ static gboolean set_sdp_services_uuid(sdp_record_t *record, HdpRole role)
 static gboolean register_service_protocols(struct hdp_adapter *adapter,
 						sdp_record_t *sdp_record)
 {
-	gboolean ret;
+	gboolean ret = TRUE;
 	uuid_t l2cap_uuid, mcap_c_uuid;
 	sdp_list_t *l2cap_list, *proto_list = NULL, *mcap_list = NULL;
 	sdp_list_t *access_proto_list = NULL;
@@ -425,11 +425,7 @@ static gboolean register_service_protocols(struct hdp_adapter *adapter,
 		goto end;
 	}
 
-	if (sdp_set_access_protos(sdp_record, access_proto_list) < 0) {
-		ret = FALSE;
-		goto end;
-	}
-	ret = TRUE;
+	sdp_set_access_protos(sdp_record, access_proto_list);
 
 end:
 	if (l2cap_list != NULL)
-- 
1.8.3.2

[PATCH 2/2] SDP: Make sdp_set_access_protos() return void

[Andrei Emeltchenko]

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

Function always returns 0, so there is sense to make it void function.
---
 lib/sdp.c     | 6 +-----
 lib/sdp_lib.h | 2 +-
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index e5e4622..da26b78 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2380,10 +2380,8 @@ static sdp_data_t *access_proto_to_dataseq(sdp_record_t *rec, sdp_list_t *proto)
  *
  * This function replaces the existing sdp_access_proto_t
  * structure (if any) with the new one specified.
- *
- * returns 0 if successful or -1 if there is a failure.
  */
-int sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *ap)
+void sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *ap)
 {
 	const sdp_list_t *p;
 	sdp_data_t *protos = NULL;
@@ -2394,8 +2392,6 @@ int sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *ap)
 	}
 
 	sdp_attr_add(rec, SDP_ATTR_PROTO_DESC_LIST, protos);
-
-	return 0;
 }
 
 int sdp_set_add_access_protos(sdp_record_t *rec, const sdp_list_t *ap)
diff --git a/lib/sdp_lib.h b/lib/sdp_lib.h
index 3ded393..72922d6 100644
--- a/lib/sdp_lib.h
+++ b/lib/sdp_lib.h
@@ -231,7 +231,7 @@ static inline int sdp_set_browse_groups(sdp_record_t *rec, sdp_list_t *seq)
 /*
  * Set the access protocols of the record to those specified in proto
  */
-int sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *proto);
+void sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *proto);
 
 /*
  * Set the additional access protocols of the record to those specified in proto
-- 
1.8.3.2

Re: [PATCH 2/2] SDP: Make sdp_set_access_protos() return void

[Marcel Holtmann]

Hi Andrei,

> Function always returns 0, so there is sense to make it void function.
> ---
> lib/sdp.c     | 6 +-----
> lib/sdp_lib.h | 2 +-
> 2 files changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/sdp.c b/lib/sdp.c
> index e5e4622..da26b78 100644
> --- a/lib/sdp.c
> +++ b/lib/sdp.c
> @@ -2380,10 +2380,8 @@ static sdp_data_t *access_proto_to_dataseq(sdp_record_t *rec, sdp_list_t *proto)
>  *
>  * This function replaces the existing sdp_access_proto_t
>  * structure (if any) with the new one specified.
> - *
> - * returns 0 if successful or -1 if there is a failure.
>  */
> -int sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *ap)
> +void sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *ap)

if this is libbluetooth API, then we can not do that. I am not planning to break that API.

Regards

Marcel

Re: [PATCH 1/2] HDP: Fix checking always constant error code

[Marcel Holtmann]

Hi Andrei,

> sdp_set_access_protos() always returns 0, there is no sense to check for
> error code. Fixes compile warnings.
> ---
> profiles/health/hdp_util.c | 8 ++------
> 1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
> index 58770b5..ff427a6 100644
> --- a/profiles/health/hdp_util.c
> +++ b/profiles/health/hdp_util.c
> @@ -362,7 +362,7 @@ static gboolean set_sdp_services_uuid(sdp_record_t *record, HdpRole role)
> static gboolean register_service_protocols(struct hdp_adapter *adapter,
> 						sdp_record_t *sdp_record)
> {
> -	gboolean ret;
> +	gboolean ret = TRUE;
> 	uuid_t l2cap_uuid, mcap_c_uuid;
> 	sdp_list_t *l2cap_list, *proto_list = NULL, *mcap_list = NULL;
> 	sdp_list_t *access_proto_list = NULL;
> @@ -425,11 +425,7 @@ static gboolean register_service_protocols(struct hdp_adapter *adapter,
> 		goto end;
> 	}
> 
> -	if (sdp_set_access_protos(sdp_record, access_proto_list) < 0) {
> -		ret = FALSE;
> -		goto end;
> -	}
> -	ret = TRUE;

might better just a comment here explaining that the call always returns success.

> +	sdp_set_access_protos(sdp_record, access_proto_list);
> 
> end:
> 	if (l2cap_list != NULL)

Regards

Marcel

Re: [PATCH 1/2] HDP: Fix checking always constant error code

[Andrei Emeltchenko]

Hi Marcel,

On Mon, Jun 09, 2014 at 02:11:14PM +0200, Marcel Holtmann wrote:
> Hi Andrei,
> 
> > sdp_set_access_protos() always returns 0, there is no sense to check for
> > error code. Fixes compile warnings.
> > ---
> > profiles/health/hdp_util.c | 8 ++------
> > 1 file changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
> > index 58770b5..ff427a6 100644
> > --- a/profiles/health/hdp_util.c
> > +++ b/profiles/health/hdp_util.c
> > @@ -362,7 +362,7 @@ static gboolean set_sdp_services_uuid(sdp_record_t *record, HdpRole role)
> > static gboolean register_service_protocols(struct hdp_adapter *adapter,
> > 						sdp_record_t *sdp_record)
> > {
> > -	gboolean ret;
> > +	gboolean ret = TRUE;
> > 	uuid_t l2cap_uuid, mcap_c_uuid;
> > 	sdp_list_t *l2cap_list, *proto_list = NULL, *mcap_list = NULL;
> > 	sdp_list_t *access_proto_list = NULL;
> > @@ -425,11 +425,7 @@ static gboolean register_service_protocols(struct hdp_adapter *adapter,
> > 		goto end;
> > 	}
> > 
> > -	if (sdp_set_access_protos(sdp_record, access_proto_list) < 0) {
> > -		ret = FALSE;
> > -		goto end;
> > -	}
> > -	ret = TRUE;
> 
> might better just a comment here explaining that the call always returns success.
> 

This function is used 51 times throughout the code without any check and
comment, I think we might need to add comments there as well.

Best regards 
Andrei Emeltchenko 

Re: [PATCH 2/2] SDP: Make sdp_set_access_protos() return void

[Andrei Emeltchenko]

Hi Marcel,

On Mon, Jun 09, 2014 at 02:10:24PM +0200, Marcel Holtmann wrote:
> Hi Andrei,
> 
> > Function always returns 0, so there is sense to make it void function.
> > ---
> > lib/sdp.c     | 6 +-----
> > lib/sdp_lib.h | 2 +-
> > 2 files changed, 2 insertions(+), 6 deletions(-)
> > 
> > diff --git a/lib/sdp.c b/lib/sdp.c
> > index e5e4622..da26b78 100644
> > --- a/lib/sdp.c
> > +++ b/lib/sdp.c
> > @@ -2380,10 +2380,8 @@ static sdp_data_t *access_proto_to_dataseq(sdp_record_t *rec, sdp_list_t *proto)
> >  *
> >  * This function replaces the existing sdp_access_proto_t
> >  * structure (if any) with the new one specified.
> > - *
> > - * returns 0 if successful or -1 if there is a failure.
> >  */
> > -int sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *ap)
> > +void sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *ap)
> 
> if this is libbluetooth API, then we can not do that. I am not planning to break that API.
> 

Is libbluetooth API internal API and only used in our code?

Best regards 
Andrei Emeltchenko

Re: [PATCH 2/2] SDP: Make sdp_set_access_protos() return void

[Marcel Holtmann]

Hi Andrei,

>>> Function always returns 0, so there is sense to make it void function.
>>> ---
>>> lib/sdp.c     | 6 +-----
>>> lib/sdp_lib.h | 2 +-
>>> 2 files changed, 2 insertions(+), 6 deletions(-)
>>> 
>>> diff --git a/lib/sdp.c b/lib/sdp.c
>>> index e5e4622..da26b78 100644
>>> --- a/lib/sdp.c
>>> +++ b/lib/sdp.c
>>> @@ -2380,10 +2380,8 @@ static sdp_data_t *access_proto_to_dataseq(sdp_record_t *rec, sdp_list_t *proto)
>>> *
>>> * This function replaces the existing sdp_access_proto_t
>>> * structure (if any) with the new one specified.
>>> - *
>>> - * returns 0 if successful or -1 if there is a failure.
>>> */
>>> -int sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *ap)
>>> +void sdp_set_access_protos(sdp_record_t *rec, const sdp_list_t *ap)
>> 
>> if this is libbluetooth API, then we can not do that. I am not planning to break that API.
>> 
> 
> Is libbluetooth API internal API and only used in our code?

it is a public API. Can be enabled with --enable-library in configure.

Regards

Marcel

[PATCH 1/2] HDP: Fix checking always constant error code

[Andrei Emeltchenko]

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

Function sdp_set_add_access_protos() always returns 0, so there is no
sense to check for error code.
---
 profiles/health/hdp_util.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/profiles/health/hdp_util.c b/profiles/health/hdp_util.c
index ff427a6..7185805 100644
--- a/profiles/health/hdp_util.c
+++ b/profiles/health/hdp_util.c
@@ -472,7 +472,7 @@ static gboolean register_service_additional_protocols(
 						struct hdp_adapter *adapter,
 						sdp_record_t *sdp_record)
 {
-	gboolean ret;
+	gboolean ret = TRUE;
 	uuid_t l2cap_uuid, mcap_d_uuid;
 	sdp_list_t *l2cap_list, *proto_list = NULL, *mcap_list = NULL;
 	sdp_list_t *access_proto_list = NULL;
@@ -523,10 +523,7 @@ static gboolean register_service_additional_protocols(
 		goto end;
 	}
 
-	if (sdp_set_add_access_protos(sdp_record, access_proto_list) < 0)
-		ret = FALSE;
-	else
-		ret = TRUE;
+	sdp_set_add_access_protos(sdp_record, access_proto_list);
 
 end:
 	if (l2cap_list != NULL)
-- 
1.8.3.2