Linux bluetooth development
 help / color / mirror / Atom feed
* who uses HCI_VENDOR_PKT
From: Pavan Savoy @ 2010-04-29 22:24 UTC (permalink / raw)
  To: linux-bluetooth

Hi,

Can I get an example usage of HCI_VENDOR_PKT ?
I want to check it against the packet format that I have come across which starts with 0x8 and with 0x9 unlike the 0xff mentioned in the HCI_VENDOR_PKT.

So, any device/links to formats ? 



^ permalink raw reply

* Re: [PATCH] Actual code fixing Sixaxis
From: Marcin Tolysz @ 2010-04-28 23:05 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: linux-input, linux-bluetooth, ospite
In-Reply-To: <1272361924.17421.6787.camel@localhost.localdomain>

On 27 April 2010 10:52, Bastien Nocera <hadess@hadess.net> wrote:
> Separate the sixaxis changes from the hid core changes.
Sixaxis changes does not mean anything without the ability to modify
the descriptor ... but sure if it helps I can separate them.

> Add a documentation file for that, don't put it in the commit log.
The documentation will need to grow for some time:
https://code.google.com/p/ghid/
https://code.google.com/p/ghid/wiki/HowTo

> Won't that trigger udev's firmware code and popup PackageKit's firmware
> downloader for each and every HID device?
I do not use this, but the solution is probably to ask PackageKit' to
ignore firmware/hid/ requests

> Can't this be done for the BT one as well, rather than requiring a
> modified firmware? (although the firmware hack could be useful as a
> debugging tool).
Sixaxis has broken HID report description(firmware?) and the only
solution to fix this is to replace it, replacing firmware works for BT
or USB devices in the same way (and sixaxis has got both descriptors
broken) (well the other way is to write a dedicated driver mirroring
the code from the general case)

The main advantage of modifying HidRD is the kernel itself does all
translations, thus it is possible to reconfigure the device by simply
changing the descriptor.
How to actually modify the descriptor? Some info and tools are on
https://code.google.com/p/ghid/ and the rest on the
http://www.usb.org/developers/hidpage/

^ permalink raw reply

* Re: [PATCH] Bug in sdp_set_supp_features solved
From: José Antonio Santos Cadenas @ 2010-04-28 22:28 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100428195127.GA24058@jh-x301>

El Wednesday 28 April 2010 21:51:27 Johan Hedberg escribió:
> Hi,
> 
> On Wed, Apr 28, 2010, José Antonio Santos Cadenas wrote:
> > From 567522ed4ac5912d967fef3017bf905591b5c24e Mon Sep 17 00:00:00 2001
> > From: Jose Antonio Santos Cadenas <santoscadenas@gmail.com>
> > Date: Wed, 28 Apr 2010 12:02:31 +0200
> > Subject: [PATCH] Bug in sdp_set_supp_features solved
> > 
> > When the data is a string or a sequence, it is not ok to dereference
> > data->val because it is already a pointer.
> > ---
> >  lib/sdp.c |   33 +++++++++++++++++++++++++++++++--
> >  1 files changed, 31 insertions(+), 2 deletions(-)
> 
> The patch has been pushed upstream with some minor changes. I had to
> edit the commit message again (try "git am" on your mail yourself and
> you'll see the diffeqrence to what got pushed upstream)

I see, sorry I sent copiying to the mail reader the patch generated with
git format-patch. I'll take it in count in the future and use git mail

> and I named the
> new variable lengths instead of sizes since the function it gets passed
> to is called sdp_seq_alloc_with_length.
> 
> Since there's no code in the bluez tree that calls this
> sdp_set_supp_features function do you perhaps have some simple code
> snippet/use case that the fixed functionality could be tested with?

I'll try to do some simple program, but we are working on HDP profile that will 
use this code.

Regards

Jose.

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

^ permalink raw reply

* obexd pull-business-card
From: Ed Tsang @ 2010-04-28 20:14 UTC (permalink / raw)
  To: Bluettooth Linux

I could not get on the same thread with Daniel and Luiz.
I am trying to get pull-business-card working.  The target is a Blackberry (with pin code ...).
When I ran it, with paring sucess...
The script will fail, ...DBusException:  org.Openobex.Error.Failed:
Dbus-monitor will 
error sender.. error_name=Openobex.Error.Failed reply_serial=4
  string ""

Then I remember on the Blackberry normally the obex service is off so I
 get into it's media menu and select "receive using bluetooth.."
The script now pass. 
Dbus-monitor 
   method call  ... serial=4
         ....
      string "d.vcf"
   method return  ... reply_serial=4

but the file is no where to find.
I tried with "default.vcf" as file name ... still no file.
My stupid question is do we need to use a predefined name "d.vcf  or default.vcf or ..?"
 Or could it be the phone don't have a default vcard?
  Or the ull-business-card is not functioning correctly?
  Any hint?

Regards, 
Ed



^ permalink raw reply

* Re: [PATCH] Update service class on adapter-by-adapter basis
From: Johan Hedberg @ 2010-04-28 20:00 UTC (permalink / raw)
  To: Elvis Pfutzenreuter; +Cc: linux-bluetooth
In-Reply-To: <1272484066-19133-1-git-send-email-epx@signove.com>

Hi,

On Wed, Apr 28, 2010, Elvis Pfutzenreuter wrote:
> When a service is added (or removed) for a specific adapter,
> only the service class of that specific adapter shall be updated.
> ---
>  src/adapter.c      |    5 +++++
>  src/adapter.h      |    2 ++
>  src/manager.c      |   16 ++--------------
>  src/manager.h      |    2 +-
>  src/sdpd-service.c |   27 +++++++++++++++++----------
>  src/sdpd.h         |    1 -
>  6 files changed, 27 insertions(+), 26 deletions(-)

Thanks for the quick response! The patch is now pushed upstream.

Johan

^ permalink raw reply

* Re: [PATCH] Bug in sdp_set_supp_features solved
From: Johan Hedberg @ 2010-04-28 19:51 UTC (permalink / raw)
  To: José Antonio Santos Cadenas; +Cc: linux-bluetooth
In-Reply-To: <201004281208.35923.jcaden@libresoft.es>

Hi,

On Wed, Apr 28, 2010, José Antonio Santos Cadenas wrote:
> From 567522ed4ac5912d967fef3017bf905591b5c24e Mon Sep 17 00:00:00 2001
> From: Jose Antonio Santos Cadenas <santoscadenas@gmail.com>
> Date: Wed, 28 Apr 2010 12:02:31 +0200
> Subject: [PATCH] Bug in sdp_set_supp_features solved
> 
> When the data is a string or a sequence, it is not ok to dereference
> data->val because it is already a pointer.
> ---
>  lib/sdp.c |   33 +++++++++++++++++++++++++++++++--
>  1 files changed, 31 insertions(+), 2 deletions(-)

The patch has been pushed upstream with some minor changes. I had to
edit the commit message again (try "git am" on your mail yourself and
you'll see the difference to what got pushed upstream) and I named the
new variable lengths instead of sizes since the function it gets passed
to is called sdp_seq_alloc_with_length.

Since there's no code in the bluez tree that calls this
sdp_set_supp_features function do you perhaps have some simple code
snippet/use case that the fixed functionality could be tested with?

Johan

^ permalink raw reply

* [PATCH] Update service class on adapter-by-adapter basis
From: Elvis Pfutzenreuter @ 2010-04-28 19:47 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: epx

When a service is added (or removed) for a specific adapter,
only the service class of that specific adapter shall be updated.
---
 src/adapter.c      |    5 +++++
 src/adapter.h      |    2 ++
 src/manager.c      |   16 ++--------------
 src/manager.h      |    2 +-
 src/sdpd-service.c |   27 +++++++++++++++++----------
 src/sdpd.h         |    1 -
 6 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index b87f7d7..f246a79 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1104,6 +1104,11 @@ void adapter_service_remove(const bdaddr_t *bdaddr, void *rec)
 	adapter_service_ins_rem(bdaddr, rec, FALSE);
 }
 
+sdp_list_t *adapter_get_services(struct btd_adapter *adapter)
+{
+	return adapter->services;
+}
+
 struct btd_device *adapter_create_device(DBusConnection *conn,
 						struct btd_adapter *adapter,
 						const char *address)
diff --git a/src/adapter.h b/src/adapter.h
index e4307d8..71d3387 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -23,6 +23,7 @@
  */
 
 #include <bluetooth/bluetooth.h>
+#include <bluetooth/sdp.h>
 #include <dbus/dbus.h>
 #include <glib.h>
 
@@ -123,6 +124,7 @@ void adapter_update_tx_power(bdaddr_t *bdaddr, uint8_t status, void *ptr);
 void adapter_update_local_name(bdaddr_t *bdaddr, uint8_t status, void *ptr);
 void adapter_service_insert(const bdaddr_t *bdaddr, void *rec);
 void adapter_service_remove(const bdaddr_t *bdaddr, void *rec);
+sdp_list_t *adapter_get_services(struct btd_adapter *adapter);
 void adapter_set_class_complete(bdaddr_t *bdaddr, uint8_t status);
 
 struct agent *adapter_get_agent(struct btd_adapter *adapter);
diff --git a/src/manager.c b/src/manager.c
index da7b91f..0c75ff7 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -61,21 +61,9 @@ const char *manager_get_base_path(void)
 	return base_path;
 }
 
-void manager_update_svc(const bdaddr_t *bdaddr, uint8_t svc)
+void manager_update_svc(struct btd_adapter* adapter, uint8_t svc)
 {
-	GSList *l;
-	bdaddr_t src;
-
-	for (l = adapters; l != NULL; l = l->next) {
-		struct btd_adapter *adapter = l->data;
-
-		adapter_get_address(adapter, &src);
-
-		if (bacmp(bdaddr, BDADDR_ANY) != 0 && bacmp(bdaddr, &src) != 0)
-			continue;
-
-		adapter_update(adapter, svc);
-	}
+	adapter_update(adapter, svc);
 }
 
 static inline DBusMessage *invalid_args(DBusMessage *msg)
diff --git a/src/manager.h b/src/manager.h
index 55d316f..6bf3e76 100644
--- a/src/manager.h
+++ b/src/manager.h
@@ -43,5 +43,5 @@ int manager_stop_adapter(int id);
 void manager_add_adapter(const char *path);
 int manager_get_default_adapter();
 void manager_set_default_adapter(int id);
-void manager_update_svc(const bdaddr_t *bdaddr, uint8_t svc);
+void manager_update_svc(struct btd_adapter *adapter, uint8_t svc);
 void btd_manager_set_offline(gboolean offline);
diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 4551577..55a30f7 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -47,11 +47,10 @@
 #include "sdpd.h"
 #include "logging.h"
 #include "manager.h"
+#include "adapter.h"
 
 static sdp_record_t *server = NULL;
 
-static uint8_t service_classes = 0x00;
-
 static uint16_t did_vendor = 0x0000;
 static uint16_t did_product = 0x0000;
 static uint16_t did_version = 0x0000;
@@ -98,9 +97,9 @@ static void update_db_timestamp(void)
 	sdp_attr_replace(server, SDP_ATTR_SVCDB_STATE, d);
 }
 
-static void update_svclass_list(const bdaddr_t *src)
+static void update_adapter_svclass_list(struct btd_adapter *adapter)
 {
-	sdp_list_t *list = sdp_get_record_list();
+	sdp_list_t *list = adapter_get_services(adapter);
 	uint8_t val = 0;
 
 	for (; list; list = list->next) {
@@ -156,14 +155,22 @@ static void update_svclass_list(const bdaddr_t *src)
 
 	SDPDBG("Service classes 0x%02x", val);
 
-	service_classes = val;
-
-	manager_update_svc(src, val);
+	manager_update_svc(adapter, val);
 }
 
-uint8_t get_service_classes(const bdaddr_t *bdaddr)
+static void update_svclass_list(const bdaddr_t *src)
 {
-	return service_classes;
+	bdaddr_t bdaddr;
+	GSList *adapters = manager_get_adapters();
+
+	for (; adapters; adapters = adapters->next) {
+		struct btd_adapter *adapter = adapters->data;
+		adapter_get_address(adapter, &bdaddr);
+
+		if (bacmp(src, BDADDR_ANY) == 0 || bacmp(src, &bdaddr) == 0)
+			update_adapter_svclass_list(adapter);
+	}
+
 }
 
 void create_ext_inquiry_response(const char *name,
@@ -613,7 +620,7 @@ success:
 	}
 
 	update_db_timestamp();
-	update_svclass_list(BDADDR_ANY);
+	update_svclass_list(&req->device);
 
 	/* Build a rsp buffer */
 	bt_put_unaligned(htonl(rec->handle), (uint32_t *) rsp->data);
diff --git a/src/sdpd.h b/src/sdpd.h
index ce189ca..e93b0b6 100644
--- a/src/sdpd.h
+++ b/src/sdpd.h
@@ -94,7 +94,6 @@ void stop_sdp_server(void);
 int add_record_to_server(const bdaddr_t *src, sdp_record_t *rec);
 int remove_record_from_server(uint32_t handle);
 
-uint8_t get_service_classes(const bdaddr_t *bdaddr);
 void create_ext_inquiry_response(const char *name,
 					int8_t tx_power, sdp_list_t *services,
 					uint8_t *data);
-- 
1.6.3.3


^ permalink raw reply related

* Re: [PATCH] Update service class on adapter-by-adapter basis
From: Johan Hedberg @ 2010-04-28 19:17 UTC (permalink / raw)
  To: Elvis Pfutzenreuter; +Cc: linux-bluetooth
In-Reply-To: <1272476798-10161-1-git-send-email-epx@signove.com>

Hi,

On Wed, Apr 28, 2010, Elvis Pfutzenreuter wrote:
> +sdp_list_t *adapter_service_list(struct btd_adapter *adapter)
> +{
> +	return adapter->services;
> +}

To make this consistent with other exported adapter functions please
call it adapter_get_services().

> +	for (; adapters; adapters = adapters->next) {
> +		adapter_get_address(adapters->data, &bdaddr);
> +
> +		if (bacmp(src, BDADDR_ANY) == 0 || bacmp(src, &bdaddr) == 0)
> +			update_adapter_svclass_list(adapters->data);
> +	}

I think it'd be good to make it explicit which type of objects are part
of the list, so add to the beginning of the loop:
	struct btd_adapter *adapter = adapters->data;

Other than those issues the patch looks ok to me.

Johan

^ permalink raw reply

* [PATCH] Update service class on adapter-by-adapter basis
From: Elvis Pfutzenreuter @ 2010-04-28 17:46 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: epx

When a service is added (or removed) for a specific adapter,
only the service class of that specific adapter shall be updated.
---
 src/adapter.c      |    5 +++++
 src/adapter.h      |    2 ++
 src/manager.c      |   16 ++--------------
 src/manager.h      |    2 +-
 src/sdpd-service.c |   26 ++++++++++++++++----------
 src/sdpd.h         |    1 -
 6 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index b87f7d7..dd79877 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1104,6 +1104,11 @@ void adapter_service_remove(const bdaddr_t *bdaddr, void *rec)
 	adapter_service_ins_rem(bdaddr, rec, FALSE);
 }
 
+sdp_list_t *adapter_service_list(struct btd_adapter *adapter)
+{
+	return adapter->services;
+}
+
 struct btd_device *adapter_create_device(DBusConnection *conn,
 						struct btd_adapter *adapter,
 						const char *address)
diff --git a/src/adapter.h b/src/adapter.h
index e4307d8..1598dea 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -23,6 +23,7 @@
  */
 
 #include <bluetooth/bluetooth.h>
+#include <bluetooth/sdp.h>
 #include <dbus/dbus.h>
 #include <glib.h>
 
@@ -123,6 +124,7 @@ void adapter_update_tx_power(bdaddr_t *bdaddr, uint8_t status, void *ptr);
 void adapter_update_local_name(bdaddr_t *bdaddr, uint8_t status, void *ptr);
 void adapter_service_insert(const bdaddr_t *bdaddr, void *rec);
 void adapter_service_remove(const bdaddr_t *bdaddr, void *rec);
+sdp_list_t *adapter_service_list(struct btd_adapter *adapter);
 void adapter_set_class_complete(bdaddr_t *bdaddr, uint8_t status);
 
 struct agent *adapter_get_agent(struct btd_adapter *adapter);
diff --git a/src/manager.c b/src/manager.c
index da7b91f..0c75ff7 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -61,21 +61,9 @@ const char *manager_get_base_path(void)
 	return base_path;
 }
 
-void manager_update_svc(const bdaddr_t *bdaddr, uint8_t svc)
+void manager_update_svc(struct btd_adapter* adapter, uint8_t svc)
 {
-	GSList *l;
-	bdaddr_t src;
-
-	for (l = adapters; l != NULL; l = l->next) {
-		struct btd_adapter *adapter = l->data;
-
-		adapter_get_address(adapter, &src);
-
-		if (bacmp(bdaddr, BDADDR_ANY) != 0 && bacmp(bdaddr, &src) != 0)
-			continue;
-
-		adapter_update(adapter, svc);
-	}
+	adapter_update(adapter, svc);
 }
 
 static inline DBusMessage *invalid_args(DBusMessage *msg)
diff --git a/src/manager.h b/src/manager.h
index 55d316f..6bf3e76 100644
--- a/src/manager.h
+++ b/src/manager.h
@@ -43,5 +43,5 @@ int manager_stop_adapter(int id);
 void manager_add_adapter(const char *path);
 int manager_get_default_adapter();
 void manager_set_default_adapter(int id);
-void manager_update_svc(const bdaddr_t *bdaddr, uint8_t svc);
+void manager_update_svc(struct btd_adapter *adapter, uint8_t svc);
 void btd_manager_set_offline(gboolean offline);
diff --git a/src/sdpd-service.c b/src/sdpd-service.c
index 4551577..23f7989 100644
--- a/src/sdpd-service.c
+++ b/src/sdpd-service.c
@@ -47,11 +47,10 @@
 #include "sdpd.h"
 #include "logging.h"
 #include "manager.h"
+#include "adapter.h"
 
 static sdp_record_t *server = NULL;
 
-static uint8_t service_classes = 0x00;
-
 static uint16_t did_vendor = 0x0000;
 static uint16_t did_product = 0x0000;
 static uint16_t did_version = 0x0000;
@@ -98,9 +97,9 @@ static void update_db_timestamp(void)
 	sdp_attr_replace(server, SDP_ATTR_SVCDB_STATE, d);
 }
 
-static void update_svclass_list(const bdaddr_t *src)
+static void update_adapter_svclass_list(struct btd_adapter *adapter)
 {
-	sdp_list_t *list = sdp_get_record_list();
+	sdp_list_t *list = adapter_service_list(adapter);
 	uint8_t val = 0;
 
 	for (; list; list = list->next) {
@@ -156,14 +155,21 @@ static void update_svclass_list(const bdaddr_t *src)
 
 	SDPDBG("Service classes 0x%02x", val);
 
-	service_classes = val;
-
-	manager_update_svc(src, val);
+	manager_update_svc(adapter, val);
 }
 
-uint8_t get_service_classes(const bdaddr_t *bdaddr)
+static void update_svclass_list(const bdaddr_t *src)
 {
-	return service_classes;
+	bdaddr_t bdaddr;
+	GSList *adapters = manager_get_adapters();
+
+	for (; adapters; adapters = adapters->next) {
+		adapter_get_address(adapters->data, &bdaddr);
+
+		if (bacmp(src, BDADDR_ANY) == 0 || bacmp(src, &bdaddr) == 0)
+			update_adapter_svclass_list(adapters->data);
+	}
+
 }
 
 void create_ext_inquiry_response(const char *name,
@@ -613,7 +619,7 @@ success:
 	}
 
 	update_db_timestamp();
-	update_svclass_list(BDADDR_ANY);
+	update_svclass_list(&req->device);
 
 	/* Build a rsp buffer */
 	bt_put_unaligned(htonl(rec->handle), (uint32_t *) rsp->data);
diff --git a/src/sdpd.h b/src/sdpd.h
index ce189ca..e93b0b6 100644
--- a/src/sdpd.h
+++ b/src/sdpd.h
@@ -94,7 +94,6 @@ void stop_sdp_server(void);
 int add_record_to_server(const bdaddr_t *src, sdp_record_t *rec);
 int remove_record_from_server(uint32_t handle);
 
-uint8_t get_service_classes(const bdaddr_t *bdaddr);
 void create_ext_inquiry_response(const char *name,
 					int8_t tx_power, sdp_list_t *services,
 					uint8_t *data);
-- 
1.6.3.3


^ permalink raw reply related

* Re: [PATCH] Add new S-frame types to l2cap parser
From: Johan Hedberg @ 2010-04-28 15:56 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth, marcel, gustavo
In-Reply-To: <1269975143-9951-1-git-send-email-padovan@profusion.mobi>

Hi,

On Tue, Mar 30, 2010, Gustavo F. Padovan wrote:
> ---
>  parser/l2cap.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/parser/l2cap.c b/parser/l2cap.c
> index 799d4b7..ad3b7ff 100644
> --- a/parser/l2cap.c
> +++ b/parser/l2cap.c
> @@ -352,8 +352,9 @@ static char *supervisory2str(uint8_t supervisory)
>  	case 0x01:
>  		return "Reject (REJ)";
>  	case 0x02:
> +		return "Receiver Not Ready (RNR)";
>  	case 0x03:
> -		return "Reserved Supervisory";
> +		return "Select Reject (SREJ)";
>  	default:
>  		return "Bad Supervisory";
>  	}

This one has now been pushed to the upstream bluez-hcidump tree. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Set l2cap mode always
From: Johan Hedberg @ 2010-04-28 15:55 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth, marcel, gustavo
In-Reply-To: <1271282625-11666-1-git-send-email-padovan@profusion.mobi>

Hi,

On Wed, Apr 14, 2010, Gustavo F. Padovan wrote:
> As ERTM can be the default mode with SOCK_STREAM we have to set
> opts.mode always.
> ---
>  test/l2test.c |    3 +--
>  1 files changed, 1 insertions(+), 2 deletions(-)
> 
> diff --git a/test/l2test.c b/test/l2test.c
> index 9413070..0bb46f3 100644
> --- a/test/l2test.c
> +++ b/test/l2test.c
> @@ -228,8 +228,7 @@ static int do_connect(char *svr)
>  	/* Set new options */
>  	opts.omtu = omtu;
>  	opts.imtu = imtu;
> -	if (rfcmode > 0)
> -		opts.mode = rfcmode;
> +	opts.mode = rfcmode;
>  
>  	opts.fcs = fcs;
>  	opts.txwin_size = txwin_size;

This one is also upstream now.

Johan

^ permalink raw reply

* Re: [PATCH 1/4] l2test: add option to use SOCK_STREAM
From: Johan Hedberg @ 2010-04-28 15:55 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth, marcel, gustavo
In-Reply-To: <1269975209-10044-1-git-send-email-padovan@profusion.mobi>

Hi,

On Tue, Mar 30, 2010, Gustavo F. Padovan wrote:
> ---
>  test/l2test.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)

All four patches have now been pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Fix bootstrap-configure command line parsing
From: Johan Hedberg @ 2010-04-28 15:41 UTC (permalink / raw)
  To: João Paulo Rechi Vita; +Cc: linux-bluetooth
In-Reply-To: <1269972441-24242-1-git-send-email-jprvita@gmail.com>

Hi,

On Tue, Mar 30, 2010, João Paulo Rechi Vita wrote:
> ---
>  bootstrap-configure |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/bootstrap-configure b/bootstrap-configure
> index 44e7ee8..5b7b88f 100755
> --- a/bootstrap-configure
> +++ b/bootstrap-configure
> @@ -8,4 +8,4 @@ fi
>      ./configure --enable-maintainer-mode \
>  		--enable-debug \
>  		--prefix=/usr \
> -		--mandir=/usr/share/man
> +		--mandir=/usr/share/man $*

This patch has now been pushed upstream. Thanks.

Johan

^ permalink raw reply

* Re: [PATCH] Update service class on adapter-by-adapter basis
From: Luiz Augusto von Dentz @ 2010-04-28 13:45 UTC (permalink / raw)
  To: Elvis Pfützenreuter; +Cc: linux-bluetooth
In-Reply-To: <1270676053-15699-1-git-send-email-epx@signove.com>

Hi Elvis,

On Thu, Apr 8, 2010 at 12:34 AM, Elvis Pfützenreuter <epx@signove.com> wrote:
> From: Elvis Pfutzenreuter <epx@signove.com>
>
> When a service is added (or removed) for a specific adapter,
> only the service class of that specific adapter shall be updated.
> ---
>  src/adapter.c      |   15 +++++++++++++++
>  src/adapter.h      |    2 ++
>  src/sdpd-service.c |   24 +++++++++++++++---------
>  src/sdpd.h         |    1 -
>  4 files changed, 32 insertions(+), 10 deletions(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index 8fc47b9..1530f83 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1103,6 +1103,21 @@ void adapter_service_remove(const bdaddr_t *bdaddr, void *rec)
>        adapter_service_ins_rem(bdaddr, rec, FALSE);
>  }
>
> +sdp_list_t *adapter_service_list(const bdaddr_t *bdaddr)
> +{
> +       struct btd_adapter *adapter;
> +       sdp_list_t *list = NULL;
> +
> +       if (bacmp(bdaddr, BDADDR_ANY) != 0) {
> +               /* Only one adapter */
> +               adapter = manager_find_adapter(bdaddr);
> +               if (adapter)
> +                       list = adapter->services;
> +       }
> +
> +       return list;
> +}

The use of BDADDR_ANY doesn't seems really necessary, it seems that
adapter_service_list will never get called passing BDADDR_ANY and if
you think about it, it doesn't really make much sense to maintain a
list for adapter any since it is just a combination of the real
adapters. Also if you have to call manager_find_adapter to resolve the
adapter pointer than it is better to place it manager.c resolve it
there and have adapter_service_list to take adapter pointer as
parameter.

>  struct btd_device *adapter_create_device(DBusConnection *conn,
>                                                struct btd_adapter *adapter,
>                                                const char *address)
> diff --git a/src/adapter.h b/src/adapter.h
> index e4307d8..5975299 100644
> --- a/src/adapter.h
> +++ b/src/adapter.h
> @@ -23,6 +23,7 @@
>  */
>
>  #include <bluetooth/bluetooth.h>
> +#include <bluetooth/sdp.h>
>  #include <dbus/dbus.h>
>  #include <glib.h>
>
> @@ -123,6 +124,7 @@ void adapter_update_tx_power(bdaddr_t *bdaddr, uint8_t status, void *ptr);
>  void adapter_update_local_name(bdaddr_t *bdaddr, uint8_t status, void *ptr);
>  void adapter_service_insert(const bdaddr_t *bdaddr, void *rec);
>  void adapter_service_remove(const bdaddr_t *bdaddr, void *rec);
> +sdp_list_t *adapter_service_list(const bdaddr_t *bdaddr);
>  void adapter_set_class_complete(bdaddr_t *bdaddr, uint8_t status);
>
>  struct agent *adapter_get_agent(struct btd_adapter *adapter);
> diff --git a/src/sdpd-service.c b/src/sdpd-service.c
> index 4551577..cddd87e 100644
> --- a/src/sdpd-service.c
> +++ b/src/sdpd-service.c
> @@ -47,11 +47,10 @@
>  #include "sdpd.h"
>  #include "logging.h"
>  #include "manager.h"
> +#include "adapter.h"
>
>  static sdp_record_t *server = NULL;
>
> -static uint8_t service_classes = 0x00;
> -
>  static uint16_t did_vendor = 0x0000;
>  static uint16_t did_product = 0x0000;
>  static uint16_t did_version = 0x0000;
> @@ -98,9 +97,9 @@ static void update_db_timestamp(void)
>        sdp_attr_replace(server, SDP_ATTR_SVCDB_STATE, d);
>  }
>
> -static void update_svclass_list(const bdaddr_t *src)
> +static void update_adapter_svclass_list(const bdaddr_t *src)
>  {
> -       sdp_list_t *list = sdp_get_record_list();
> +       sdp_list_t *list = adapter_service_list(src);
>        uint8_t val = 0;
>
>        for (; list; list = list->next) {
> @@ -156,14 +155,21 @@ static void update_svclass_list(const bdaddr_t *src)
>
>        SDPDBG("Service classes 0x%02x", val);
>
> -       service_classes = val;
> -
>        manager_update_svc(src, val);
>  }
>
> -uint8_t get_service_classes(const bdaddr_t *bdaddr)
> +static void update_svclass_list(const bdaddr_t *src)
>  {
> -       return service_classes;
> +       bdaddr_t bdaddr;
> +       GSList *adapters = manager_get_adapters();
> +
> +       for (; adapters; adapters = adapters->next) {
> +               adapter_get_address(adapters->data, &bdaddr);
> +
> +               if (bacmp(src, BDADDR_ANY) == 0 || bacmp(src, &bdaddr) == 0)
> +                       update_adapter_svclass_list(&bdaddr);
> +       }
> +
>  }
>
>  void create_ext_inquiry_response(const char *name,
> @@ -613,7 +619,7 @@ success:
>        }
>
>        update_db_timestamp();
> -       update_svclass_list(BDADDR_ANY);
> +       update_svclass_list(&req->device);
>
>        /* Build a rsp buffer */
>        bt_put_unaligned(htonl(rec->handle), (uint32_t *) rsp->data);
> diff --git a/src/sdpd.h b/src/sdpd.h
> index 1f0a229..d496a4a 100644
> --- a/src/sdpd.h
> +++ b/src/sdpd.h
> @@ -93,7 +93,6 @@ void stop_sdp_server(void);
>  int add_record_to_server(const bdaddr_t *src, sdp_record_t *rec);
>  int remove_record_from_server(uint32_t handle);
>
> -uint8_t get_service_classes(const bdaddr_t *bdaddr);
>  void create_ext_inquiry_response(const char *name,
>                                        int8_t tx_power, sdp_list_t *services,
>                                        uint8_t *data);
> --
> 1.6.3.3
>
> --
> 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
>



-- 
Luiz Augusto von Dentz
Computer Engineer

^ permalink raw reply

* Re: [PATCH] Bug in sdp_set_supp_features solved
From: José Antonio Santos Cadenas @ 2010-04-28 10:11 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <201004281208.35923.jcaden@libresoft.es>

El Wednesday 28 April 2010 12:08:35 José Antonio Santos Cadenas escribió:
> From 567522ed4ac5912d967fef3017bf905591b5c24e Mon Sep 17 00:00:00 2001
> From: Jose Antonio Santos Cadenas <santoscadenas@gmail.com>
> Date: Wed, 28 Apr 2010 12:02:31 +0200
> Subject: [PATCH] Bug in sdp_set_supp_features solved
> 
> When the data is a string or a sequence, it is not ok to dereference
> data->val because it is already a pointer.
Also sizes are added because the strings are not terminated in '\0' and otherwise 
it is not possible to know its size.
> ---
>  lib/sdp.c |   33 +++++++++++++++++++++++++++++++--
>  1 files changed, 31 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/sdp.c b/lib/sdp.c
> index 5f1f2fc..f9a6541 100644
> --- a/lib/sdp.c
> +++ b/lib/sdp.c
> @@ -4709,6 +4709,7 @@ int sdp_set_supp_feat(sdp_record_t *rec, const sdp_list_t *sf)
>  	for (p = sf, i = 0; p; p = p->next, i++) {
>  		int plen, j;
>  		void **dtds, **vals;
> +		int *sizes;
> 
>  		plen = sdp_list_len(p->data);
>  		dtds = malloc(plen * sizeof(void *));
> @@ -4719,14 +4720,42 @@ int sdp_set_supp_feat(sdp_record_t *rec, const sdp_list_t *sf)
>  			free(dtds);
>  			goto fail;
>  		}
> +		sizes = malloc(plen * sizeof(int *));
> +		if (!sizes) {
> +			free(dtds);
> +			free(vals);
> +			goto fail;
> +		}
>  		for (r = p->data, j = 0; r; r = r->next, j++) {
>  			sdp_data_t *data = (sdp_data_t*)r->data;
>  			dtds[j] = &data->dtd;
> -			vals[j] = &data->val;
> +			switch (data->dtd) {
> +			case SDP_URL_STR8:
> +			case SDP_URL_STR16:
> +			case SDP_TEXT_STR8:
> +			case SDP_TEXT_STR16:
> +				vals[j] = data->val.str;
> +				sizes[j] = data->unitSize - sizeof(uint8_t);
> +				break;
> +			case SDP_ALT8:
> +			case SDP_ALT16:
> +			case SDP_ALT32:
> +			case SDP_SEQ8:
> +			case SDP_SEQ16:
> +			case SDP_SEQ32:
> +				vals[j] = data->val.dataseq;
> +				sizes[j] = 0;
> +				break;
> +			default:
> +				vals[j] = &data->val;
> +				sizes[j] = 0;
> +				break;
> +			}
>  		}
> -		feat = sdp_seq_alloc(dtds, vals, plen);
> +		feat = sdp_seq_alloc_with_length(dtds, vals, sizes, plen);
>  		free(dtds);
>  		free(vals);
> +		free(sizes);
>  		if (!feat)
>  			goto fail;
>  		seqDTDs[i] = &feat->dtd;
> --
> 1.6.3.3
> 
> --
> 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
> 

^ permalink raw reply

* [PATCH] Bug in sdp_set_supp_features solved
From: José Antonio Santos Cadenas @ 2010-04-28 10:08 UTC (permalink / raw)
  To: linux-bluetooth

>From 567522ed4ac5912d967fef3017bf905591b5c24e Mon Sep 17 00:00:00 2001
From: Jose Antonio Santos Cadenas <santoscadenas@gmail.com>
Date: Wed, 28 Apr 2010 12:02:31 +0200
Subject: [PATCH] Bug in sdp_set_supp_features solved

When the data is a string or a sequence, it is not ok to dereference
data->val because it is already a pointer.
---
 lib/sdp.c |   33 +++++++++++++++++++++++++++++++--
 1 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index 5f1f2fc..f9a6541 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -4709,6 +4709,7 @@ int sdp_set_supp_feat(sdp_record_t *rec, const sdp_list_t *sf)
 	for (p = sf, i = 0; p; p = p->next, i++) {
 		int plen, j;
 		void **dtds, **vals;
+		int *sizes;

 		plen = sdp_list_len(p->data);
 		dtds = malloc(plen * sizeof(void *));
@@ -4719,14 +4720,42 @@ int sdp_set_supp_feat(sdp_record_t *rec, const sdp_list_t *sf)
 			free(dtds);
 			goto fail;
 		}
+		sizes = malloc(plen * sizeof(int *));
+		if (!sizes) {
+			free(dtds);
+			free(vals);
+			goto fail;
+		}
 		for (r = p->data, j = 0; r; r = r->next, j++) {
 			sdp_data_t *data = (sdp_data_t*)r->data;
 			dtds[j] = &data->dtd;
-			vals[j] = &data->val;
+			switch (data->dtd) {
+			case SDP_URL_STR8:
+			case SDP_URL_STR16:
+			case SDP_TEXT_STR8:
+			case SDP_TEXT_STR16:
+				vals[j] = data->val.str;
+				sizes[j] = data->unitSize - sizeof(uint8_t);
+				break;
+			case SDP_ALT8:
+			case SDP_ALT16:
+			case SDP_ALT32:
+			case SDP_SEQ8:
+			case SDP_SEQ16:
+			case SDP_SEQ32:
+				vals[j] = data->val.dataseq;
+				sizes[j] = 0;
+				break;
+			default:
+				vals[j] = &data->val;
+				sizes[j] = 0;
+				break;
+			}
 		}
-		feat = sdp_seq_alloc(dtds, vals, plen);
+		feat = sdp_seq_alloc_with_length(dtds, vals, sizes, plen);
 		free(dtds);
 		free(vals);
+		free(sizes);
 		if (!feat)
 			goto fail;
 		seqDTDs[i] = &feat->dtd;
--
1.6.3.3


^ permalink raw reply related

* Re: [PATCH] Added memory checks whenever memory is allocated in SDP
From: Santiago Carot-Nemesio @ 2010-04-28  7:53 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100428074642.GA30577@jh-x301>

Hi,

El mié, 28-04-2010 a las 10:46 +0300, Johan Hedberg escribió:
> Hi,
> 
> On Wed, Apr 28, 2010, Santiago Carot-Nemesio wrote:
> > This patch checks memory allocated with malloc and removes uneccessary (**void) castings.
> > 
> > >From d80347d56e730388d1eca190e60c3c09ac62db27 Mon Sep 17 00:00:00 2001
> > From: Santiago Carot-Nemesio <sancane@gmail.com>
> > Date: Wed, 28 Apr 2010 09:10:24 +0200
> > Subject: [PATCH] Added memory checks whenever memory is allocated in SDP
> > 
> > ---
> >  lib/sdp.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
> >  1 files changed, 77 insertions(+), 10 deletions(-)
> 
> Thanks. This one is now pushed upstream after (still) some manual
> editing of the commit message for it to not contain longer lines than 74
> characters (the lines should be short enough for them to fit into a
> 80-character wide terminal when doing "git log").
> 

Ok, I'll keep it in mind for next time.

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



^ permalink raw reply

* Re: [PATCH] Added memory checks whenever memory is allocated in SDP
From: Johan Hedberg @ 2010-04-28  7:46 UTC (permalink / raw)
  To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
In-Reply-To: <1272439009.2203.3.camel@mosquito>

Hi,

On Wed, Apr 28, 2010, Santiago Carot-Nemesio wrote:
> This patch checks memory allocated with malloc and removes uneccessary (**void) castings.
> 
> >From d80347d56e730388d1eca190e60c3c09ac62db27 Mon Sep 17 00:00:00 2001
> From: Santiago Carot-Nemesio <sancane@gmail.com>
> Date: Wed, 28 Apr 2010 09:10:24 +0200
> Subject: [PATCH] Added memory checks whenever memory is allocated in SDP
> 
> ---
>  lib/sdp.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 files changed, 77 insertions(+), 10 deletions(-)

Thanks. This one is now pushed upstream after (still) some manual
editing of the commit message for it to not contain longer lines than 74
characters (the lines should be short enough for them to fit into a
80-character wide terminal when doing "git log").

Johan

^ permalink raw reply

* [PATCH] Added memory checks whenever memory is allocated in SDP
From: Santiago Carot-Nemesio @ 2010-04-28  7:16 UTC (permalink / raw)
  To: linux-bluetooth

This patch checks memory allocated with malloc and removes uneccessary (**void) castings.

>>From d80347d56e730388d1eca190e60c3c09ac62db27 Mon Sep 17 00:00:00 2001
From: Santiago Carot-Nemesio <sancane@gmail.com>
Date: Wed, 28 Apr 2010 09:10:24 +0200
Subject: [PATCH] Added memory checks whenever memory is allocated in SDP

---
 lib/sdp.c |   87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
 1 files changed, 77 insertions(+), 10 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index 667d412..63dcabf 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1078,6 +1078,8 @@ static sdp_data_t *extract_int(const void *p, int bufsize, int *len)
 	}
 
 	d = malloc(sizeof(sdp_data_t));
+	if (!d)
+		return NULL;
 
 	SDPDBG("Extracting integer\n");
 	memset(d, 0, sizeof(sdp_data_t));
@@ -1152,6 +1154,9 @@ static sdp_data_t *extract_uuid(const uint8_t *p, int bufsize, int *len,
 {
 	sdp_data_t *d = malloc(sizeof(sdp_data_t));
 
+	if (!d)
+		return NULL;
+
 	SDPDBG("Extracting UUID");
 	memset(d, 0, sizeof(sdp_data_t));
 	if (sdp_uuid_extract(p, bufsize, &d->val.uuid, len) < 0) {
@@ -1179,6 +1184,8 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
 	}
 
 	d = malloc(sizeof(sdp_data_t));
+	if (!d)
+		return NULL;
 
 	memset(d, 0, sizeof(sdp_data_t));
 	d->dtd = *(uint8_t *) p;
@@ -1302,6 +1309,9 @@ static sdp_data_t *extract_seq(const void *p, int bufsize, int *len,
 	sdp_data_t *curr, *prev;
 	sdp_data_t *d = malloc(sizeof(sdp_data_t));
 
+	if (!d)
+		return NULL;
+
 	SDPDBG("Extracting SEQ");
 	memset(d, 0, sizeof(sdp_data_t));
 	*len = sdp_extract_seqtype(p, bufsize, &d->dtd, &seqlen);
@@ -1945,10 +1955,15 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
 		sdp_data_t *d;
 		for (d = sdpdata->val.dataseq; d; d = d->next) {
 			uuid_t *u;
-			if (d->dtd < SDP_UUID16 || d->dtd > SDP_UUID128)
+			if (d->dtd < SDP_UUID16 || d->dtd > SDP_UUID128) {
+				errno = EINVAL;
 				goto fail;
+			}
 
 			u = malloc(sizeof(uuid_t));
+			if (!u)
+				goto fail;
+
 			memset(u, 0, sizeof(uuid_t));
 			*u = d->val.uuid;
 			*seqp = sdp_list_append(*seqp, u);
@@ -1957,7 +1972,7 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
 	}
 fail:
 	sdp_list_free(*seqp, free);
-	errno = EINVAL;
+	*seqp = NULL;
 	return -1;
 }
 
@@ -1973,8 +1988,16 @@ int sdp_set_uuidseq_attr(sdp_record_t *rec, uint16_t aid, sdp_list_t *seq)
 	len = sdp_list_len(seq);
 	if (!seq || len == 0)
 		return -1;
-	dtds = (void **)malloc(len * sizeof(void *));
-	values = (void **)malloc(len * sizeof(void *));
+	dtds = malloc(len * sizeof(void *));
+	if (!dtds)
+		return -1;
+
+	values = malloc(len * sizeof(void *));
+	if (!values) {
+		free(dtds);
+		return -1;
+	}
+
 	for (p = seq, i = 0; i < len; i++, p = p->next) {
 		uuid_t *uuid = (uuid_t *)p->data;
 		if (uuid)
@@ -2028,6 +2051,11 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
 		sdp_data_t *pOffset = pEncoding->next;
 		if (pEncoding && pOffset) {
 			lang = malloc(sizeof(sdp_lang_attr_t));
+			if (!lang) {
+				sdp_list_free(*langSeq, free);
+				*langSeq = NULL;
+				return -1;
+			}
 			lang->code_ISO639 = pCode->val.uint16;
 			lang->encoding = pEncoding->val.uint16;
 			lang->base_offset = pOffset->val.uint16;
@@ -2069,6 +2097,11 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
 
 		if (uuid != NULL) {
 			profDesc = malloc(sizeof(sdp_profile_desc_t));
+			if (!profDesc) {
+				sdp_list_free(*profDescSeq, free);
+				*profDescSeq = NULL;
+				return -1;
+			}
 			profDesc->uuid = *uuid;
 			profDesc->version = version;
 #ifdef SDP_DEBUG
@@ -2230,8 +2263,16 @@ static sdp_data_t *access_proto_to_dataseq(sdp_record_t *rec, sdp_list_t *proto)
 	sdp_list_t *p;
 
 	seqlen = sdp_list_len(proto);
-	seqDTDs = (void **)malloc(seqlen * sizeof(void *));
-	seqs = (void **)malloc(seqlen * sizeof(void *));
+	seqDTDs = malloc(seqlen * sizeof(void *));
+	if (!seqDTDs)
+		return NULL;
+
+	seqs = malloc(seqlen * sizeof(void *));
+	if (!seqs) {
+		free(seqDTDs);
+		return NULL;
+	}
+
 	for (i = 0, p = proto; p; p = p->next, i++) {
 		sdp_list_t *elt = (sdp_list_t *)p->data;
 		sdp_data_t *s;
@@ -2350,10 +2391,19 @@ int sdp_set_lang_attr(sdp_record_t *rec, const sdp_list_t *seq)
 {
 	uint8_t uint16 = SDP_UINT16;
 	int status = 0, i = 0, seqlen = sdp_list_len(seq);
-	void **dtds = (void **)malloc(3 * seqlen * sizeof(void *));
-	void **values = (void **)malloc(3 * seqlen * sizeof(void *));
+	void **dtds, **values;
 	const sdp_list_t *p;
 
+	dtds = malloc(3 * seqlen * sizeof(void *));
+	if (!dtds)
+		return -1;
+
+	values = malloc(3 * seqlen * sizeof(void *));
+	if (!values) {
+		free(dtds);
+		return -1;
+	}
+
 	for (p = seq; p; p = p->next) {
 		sdp_lang_attr_t *lang = (sdp_lang_attr_t *)p->data;
 		if (!lang) {
@@ -2455,10 +2505,19 @@ int sdp_set_profile_descs(sdp_record_t *rec, const sdp_list_t *profiles)
 	uint8_t uuid128 = SDP_UUID128;
 	uint8_t uint16 = SDP_UINT16;
 	int i = 0, seqlen = sdp_list_len(profiles);
-	void **seqDTDs = (void **)malloc(seqlen * sizeof(void *));
-	void **seqs = (void **)malloc(seqlen * sizeof(void *));
+	void **seqDTDs, **seqs;
 	const sdp_list_t *p;
 
+	seqDTDs = malloc(seqlen * sizeof(void *));
+	if (!seqDTDs)
+		return -1;
+
+	seqs = malloc(seqlen * sizeof(void *));
+	if (!seqs) {
+		free(seqDTDs);
+		return -1;
+	}
+
 	for (p = profiles; p; p = p->next) {
 		sdp_data_t *seq;
 		void *dtds[2], *values[2];
@@ -2643,6 +2702,10 @@ void sdp_uuid32_to_uuid128(uuid_t *uuid128, uuid_t *uuid32)
 uuid_t *sdp_uuid_to_uuid128(uuid_t *uuid)
 {
 	uuid_t *uuid128 = bt_malloc(sizeof(uuid_t));
+
+	if (!uuid128)
+		return NULL;
+
 	memset(uuid128, 0, sizeof(uuid_t));
 	switch (uuid->type) {
 	case SDP_UUID128:
@@ -3087,6 +3150,10 @@ int sdp_record_update(sdp_session_t *session, const sdp_record_t *rec)
 sdp_record_t *sdp_record_alloc()
 {
 	sdp_record_t *rec = malloc(sizeof(sdp_record_t));
+
+	if (!rec)
+		return NULL;
+
 	memset((void *)rec, 0, sizeof(sdp_record_t));
 	rec->handle = 0xffffffff;
 	return rec;
-- 
1.6.3.3


^ permalink raw reply related

* Re: [patch v2] bluetooth: handle l2cap_create_connless_pdu() errors
From: David Miller @ 2010-04-28  0:03 UTC (permalink / raw)
  To: marcel
  Cc: gustavo, error27, andrei.emeltchenko, linux-bluetooth, netdev,
	kernel-janitors
In-Reply-To: <1272333926.22838.116.camel@localhost.localdomain>

From: Marcel Holtmann <marcel@holtmann.org>
Date: Tue, 27 Apr 2010 04:05:26 +0200

> my bad here. I looked at the patch and thought it was only an issue
> inside the development tree. So was letting Gustavo sort this out. You
> are 100% right, this needs to be fixed right away. If you wanna take
> this directly, then
> 
> Acked-by: Marcel Holtmann <marcel@holtmann.org>

I applied the original patch with your ACK, thanks Marcel.

^ permalink raw reply

* Re: [PATCH 1/2] Added checks in memory allocated with malloc in SDP
From: Marcel Holtmann @ 2010-04-27 19:21 UTC (permalink / raw)
  To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
In-Reply-To: <1272395759.1914.3.camel@mosquito>

Hi Santiago,

> This patch add checks in memory allocated using malloc in SDP. 
> 
> >From b19940a05fcd341b1197606661320fa91ba19390 Mon Sep 17 00:00:00 2001
> From: Santiago Carot-Nemesio <sancane@gmail.com>
> Date: Tue, 27 Apr 2010 20:44:18 +0200
> Subject: [PATCH 1/2] Added checks in memory allocated with malloc in SDP
> 
> ---
>  lib/sdp.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 76 insertions(+), 6 deletions(-)
>  mode change 100644 => 100755 lib/sdp.c
> 
> diff --git a/lib/sdp.c b/lib/sdp.c
> old mode 100644
> new mode 100755
> index 667d412..0def315
> --- a/lib/sdp.c
> +++ b/lib/sdp.c
> @@ -1078,6 +1078,8 @@ static sdp_data_t *extract_int(const void *p, int bufsize, int *len)
>  	}
>  
>  	d = malloc(sizeof(sdp_data_t));
> +	if (!d)
> +		return NULL;
>  
>  	SDPDBG("Extracting integer\n");
>  	memset(d, 0, sizeof(sdp_data_t));
> @@ -1152,6 +1154,9 @@ static sdp_data_t *extract_uuid(const uint8_t *p, int bufsize, int *len,
>  {
>  	sdp_data_t *d = malloc(sizeof(sdp_data_t));
>  
> +	if (!d)
> +		return NULL;
> +
>  	SDPDBG("Extracting UUID");
>  	memset(d, 0, sizeof(sdp_data_t));
>  	if (sdp_uuid_extract(p, bufsize, &d->val.uuid, len) < 0) {
> @@ -1179,6 +1184,8 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
>  	}
>  
>  	d = malloc(sizeof(sdp_data_t));
> +	if (!d)
> +		return NULL;
>  
>  	memset(d, 0, sizeof(sdp_data_t));
>  	d->dtd = *(uint8_t *) p;
> @@ -1302,6 +1309,9 @@ static sdp_data_t *extract_seq(const void *p, int bufsize, int *len,
>  	sdp_data_t *curr, *prev;
>  	sdp_data_t *d = malloc(sizeof(sdp_data_t));
>  
> +	if (!d)
> +		return NULL;
> +
>  	SDPDBG("Extracting SEQ");
>  	memset(d, 0, sizeof(sdp_data_t));
>  	*len = sdp_extract_seqtype(p, bufsize, &d->dtd, &seqlen);
> @@ -1945,10 +1955,15 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
>  		sdp_data_t *d;
>  		for (d = sdpdata->val.dataseq; d; d = d->next) {
>  			uuid_t *u;
> -			if (d->dtd < SDP_UUID16 || d->dtd > SDP_UUID128)
> +			if (d->dtd < SDP_UUID16 || d->dtd > SDP_UUID128) {
> +				errno = EINVAL;
>  				goto fail;
> +			}
>  
>  			u = malloc(sizeof(uuid_t));
> +			if (!u)
> +				goto fail;
> +
>  			memset(u, 0, sizeof(uuid_t));
>  			*u = d->val.uuid;
>  			*seqp = sdp_list_append(*seqp, u);
> @@ -1957,7 +1972,7 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
>  	}
>  fail:
>  	sdp_list_free(*seqp, free);
> -	errno = EINVAL;
> +	*seqp = NULL;
>  	return -1;
>  }
>  
> @@ -1974,7 +1989,15 @@ int sdp_set_uuidseq_attr(sdp_record_t *rec, uint16_t aid, sdp_list_t *seq)
>  	if (!seq || len == 0)
>  		return -1;
>  	dtds = (void **)malloc(len * sizeof(void *));
> +	if (!dtds)
> +		return -1;
> +
>  	values = (void **)malloc(len * sizeof(void *));
> +	if (!values) {
> +		free(dtds);
> +		return -1;
> +	}
> +

since you are touching this code, remove the pointless (void **) casting
for malloc.

>  	for (p = seq, i = 0; i < len; i++, p = p->next) {
>  		uuid_t *uuid = (uuid_t *)p->data;
>  		if (uuid)
> @@ -2028,6 +2051,9 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
>  		sdp_data_t *pOffset = pEncoding->next;
>  		if (pEncoding && pOffset) {
>  			lang = malloc(sizeof(sdp_lang_attr_t));
> +			if (!lang)
> +				goto fail;
> +
>  			lang->code_ISO639 = pCode->val.uint16;
>  			lang->encoding = pEncoding->val.uint16;
>  			lang->base_offset = pOffset->val.uint16;
> @@ -2039,6 +2065,10 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
>  		curr_data = pOffset->next;
>  	}
>  	return 0;
> +fail:
> +	sdp_list_free(*langSeq, free);
> +	*langSeq = NULL;
> +	return -1;
>  }

Why a goto here. There is only one user.
 
>  int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
> @@ -2069,6 +2099,8 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
>  
>  		if (uuid != NULL) {
>  			profDesc = malloc(sizeof(sdp_profile_desc_t));
> +			if (!profDesc)
> +				goto fail;
>  			profDesc->uuid = *uuid;
>  			profDesc->version = version;
>  #ifdef SDP_DEBUG
> @@ -2079,6 +2111,10 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
>  		}
>  	}
>  	return 0;
> +fail:
> +	sdp_list_free(*profDescSeq, free);
> +	*profDescSeq = NULL;
> +	return -1;
>  }

Same here. Only one user. So better keep the the error handling in
place.
 
>  int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
> @@ -2231,7 +2267,15 @@ static sdp_data_t *access_proto_to_dataseq(sdp_record_t *rec, sdp_list_t *proto)
>  
>  	seqlen = sdp_list_len(proto);
>  	seqDTDs = (void **)malloc(seqlen * sizeof(void *));
> +	if (!seqDTDs)
> +		return NULL;
> +
>  	seqs = (void **)malloc(seqlen * sizeof(void *));
> +	if (!seqs) {
> +		free(seqDTDs);
> +		return NULL;
> +	}
> +

Remove the casting while at it.

> @@ -2350,10 +2394,19 @@ int sdp_set_lang_attr(sdp_record_t *rec, const sdp_list_t *seq)
>  {
>  	uint8_t uint16 = SDP_UINT16;
>  	int status = 0, i = 0, seqlen = sdp_list_len(seq);
> -	void **dtds = (void **)malloc(3 * seqlen * sizeof(void *));
> -	void **values = (void **)malloc(3 * seqlen * sizeof(void *));
> +	void **dtds, **values;
>  	const sdp_list_t *p;
>  
> +	dtds = (void **)malloc(3 * seqlen * sizeof(void *));
> +	if (!dtds)
> +		return -1;
> +
> +	values = (void **)malloc(3 * seqlen * sizeof(void *));
> +	if (!values) {
> +		free(dtds);
> +		return -1;
> +	}
> +

Don't re-introduce this casting. It is pointless.

Regards

Marcel



^ permalink raw reply

* [PATCH 2/2] Removed unnecessary assignment of ENOMEM to errno when malloc fails in SDP
From: Santiago Carot-Nemesio @ 2010-04-27 19:20 UTC (permalink / raw)
  To: linux-bluetooth
In-Reply-To: <1272395759.1914.3.camel@mosquito>

This patch remove unnecessary assignment of ENOMEM when malloc fails

>>From 51fd53c35a860910ddcaf2d528ddd1978484468c Mon Sep 17 00:00:00 2001
From: Santiago Carot-Nemesio <sancane@gmail.com>
Date: Tue, 27 Apr 2010 21:03:36 +0200
Subject: [PATCH 2/2] Removed unnecessary assignment of ENOMEM to errno when malloc fails in SDP

---
 lib/sdp.c |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/lib/sdp.c b/lib/sdp.c
index 0def315..ec194a4 100755
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -2865,7 +2865,6 @@ int sdp_device_record_register_binary(sdp_session_t *session, bdaddr_t *device,
 	rsp = malloc(SDP_RSP_BUFFER_SIZE);
 	if (req == NULL || rsp == NULL) {
 		status = -1;
-		errno = ENOMEM;
 		goto end;
 	}
 
@@ -2992,7 +2991,6 @@ int sdp_device_record_unregister_binary(sdp_session_t *session, bdaddr_t *device
 	reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
 	rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
 	if (!reqbuf || !rspbuf) {
-		errno = ENOMEM;
 		status = -1;
 		goto end;
 	}
@@ -3087,7 +3085,6 @@ int sdp_device_record_update(sdp_session_t *session, bdaddr_t *device, const sdp
 	reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
 	rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
 	if (!reqbuf || !rspbuf) {
-		errno = ENOMEM;
 		status = -1;
 		goto end;
 	}
@@ -3369,7 +3366,6 @@ int sdp_service_search_req(sdp_session_t *session, const sdp_list_t *search,
 	reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
 	rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
 	if (!reqbuf || !rspbuf) {
-		errno = ENOMEM;
 		status = -1;
 		goto end;
 	}
@@ -3543,10 +3539,9 @@ sdp_record_t *sdp_service_attr_req(sdp_session_t *session, uint32_t handle,
 
 	reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
 	rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
-	if (!reqbuf || !rspbuf) {
-		errno = ENOMEM;
+	if (!reqbuf || !rspbuf)
 		goto end;
-	}
+
 	reqhdr = (sdp_pdu_hdr_t *) reqbuf;
 	reqhdr->pdu_id = SDP_SVC_ATTR_REQ;
 
@@ -3692,10 +3687,9 @@ sdp_session_t *sdp_create(int sk, uint32_t flags)
 	struct sdp_transaction *t;
 
 	session = malloc(sizeof(sdp_session_t));
-	if (!session) {
-		errno = ENOMEM;
+	if (!session)
 		return NULL;
-	}
+
 	memset(session, 0, sizeof(*session));
 
 	session->flags = flags;
@@ -3703,7 +3697,6 @@ sdp_session_t *sdp_create(int sk, uint32_t flags)
 
 	t = malloc(sizeof(struct sdp_transaction));
 	if (!t) {
-		errno = ENOMEM;
 		free(session);
 		return NULL;
 	}
@@ -4365,7 +4358,6 @@ int sdp_service_search_attr_req(sdp_session_t *session, const sdp_list_t *search
 	reqbuf = malloc(SDP_REQ_BUFFER_SIZE);
 	rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
 	if (!reqbuf || !rspbuf) {
-		errno = ENOMEM;
 		status = -1;
 		goto end;
 	}
-- 
1.6.3.3


^ permalink raw reply related

* [PATCH 1/2] Added checks in memory allocated with malloc in SDP
From: Santiago Carot-Nemesio @ 2010-04-27 19:15 UTC (permalink / raw)
  To: linux-bluetooth

This patch add checks in memory allocated using malloc in SDP. 

>>From b19940a05fcd341b1197606661320fa91ba19390 Mon Sep 17 00:00:00 2001
From: Santiago Carot-Nemesio <sancane@gmail.com>
Date: Tue, 27 Apr 2010 20:44:18 +0200
Subject: [PATCH 1/2] Added checks in memory allocated with malloc in SDP

---
 lib/sdp.c |   82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 76 insertions(+), 6 deletions(-)
 mode change 100644 => 100755 lib/sdp.c

diff --git a/lib/sdp.c b/lib/sdp.c
old mode 100644
new mode 100755
index 667d412..0def315
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1078,6 +1078,8 @@ static sdp_data_t *extract_int(const void *p, int bufsize, int *len)
 	}
 
 	d = malloc(sizeof(sdp_data_t));
+	if (!d)
+		return NULL;
 
 	SDPDBG("Extracting integer\n");
 	memset(d, 0, sizeof(sdp_data_t));
@@ -1152,6 +1154,9 @@ static sdp_data_t *extract_uuid(const uint8_t *p, int bufsize, int *len,
 {
 	sdp_data_t *d = malloc(sizeof(sdp_data_t));
 
+	if (!d)
+		return NULL;
+
 	SDPDBG("Extracting UUID");
 	memset(d, 0, sizeof(sdp_data_t));
 	if (sdp_uuid_extract(p, bufsize, &d->val.uuid, len) < 0) {
@@ -1179,6 +1184,8 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
 	}
 
 	d = malloc(sizeof(sdp_data_t));
+	if (!d)
+		return NULL;
 
 	memset(d, 0, sizeof(sdp_data_t));
 	d->dtd = *(uint8_t *) p;
@@ -1302,6 +1309,9 @@ static sdp_data_t *extract_seq(const void *p, int bufsize, int *len,
 	sdp_data_t *curr, *prev;
 	sdp_data_t *d = malloc(sizeof(sdp_data_t));
 
+	if (!d)
+		return NULL;
+
 	SDPDBG("Extracting SEQ");
 	memset(d, 0, sizeof(sdp_data_t));
 	*len = sdp_extract_seqtype(p, bufsize, &d->dtd, &seqlen);
@@ -1945,10 +1955,15 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
 		sdp_data_t *d;
 		for (d = sdpdata->val.dataseq; d; d = d->next) {
 			uuid_t *u;
-			if (d->dtd < SDP_UUID16 || d->dtd > SDP_UUID128)
+			if (d->dtd < SDP_UUID16 || d->dtd > SDP_UUID128) {
+				errno = EINVAL;
 				goto fail;
+			}
 
 			u = malloc(sizeof(uuid_t));
+			if (!u)
+				goto fail;
+
 			memset(u, 0, sizeof(uuid_t));
 			*u = d->val.uuid;
 			*seqp = sdp_list_append(*seqp, u);
@@ -1957,7 +1972,7 @@ int sdp_get_uuidseq_attr(const sdp_record_t *rec, uint16_t attr,
 	}
 fail:
 	sdp_list_free(*seqp, free);
-	errno = EINVAL;
+	*seqp = NULL;
 	return -1;
 }
 
@@ -1974,7 +1989,15 @@ int sdp_set_uuidseq_attr(sdp_record_t *rec, uint16_t aid, sdp_list_t *seq)
 	if (!seq || len == 0)
 		return -1;
 	dtds = (void **)malloc(len * sizeof(void *));
+	if (!dtds)
+		return -1;
+
 	values = (void **)malloc(len * sizeof(void *));
+	if (!values) {
+		free(dtds);
+		return -1;
+	}
+
 	for (p = seq, i = 0; i < len; i++, p = p->next) {
 		uuid_t *uuid = (uuid_t *)p->data;
 		if (uuid)
@@ -2028,6 +2051,9 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
 		sdp_data_t *pOffset = pEncoding->next;
 		if (pEncoding && pOffset) {
 			lang = malloc(sizeof(sdp_lang_attr_t));
+			if (!lang)
+				goto fail;
+
 			lang->code_ISO639 = pCode->val.uint16;
 			lang->encoding = pEncoding->val.uint16;
 			lang->base_offset = pOffset->val.uint16;
@@ -2039,6 +2065,10 @@ int sdp_get_lang_attr(const sdp_record_t *rec, sdp_list_t **langSeq)
 		curr_data = pOffset->next;
 	}
 	return 0;
+fail:
+	sdp_list_free(*langSeq, free);
+	*langSeq = NULL;
+	return -1;
 }
 
 int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
@@ -2069,6 +2099,8 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
 
 		if (uuid != NULL) {
 			profDesc = malloc(sizeof(sdp_profile_desc_t));
+			if (!profDesc)
+				goto fail;
 			profDesc->uuid = *uuid;
 			profDesc->version = version;
 #ifdef SDP_DEBUG
@@ -2079,6 +2111,10 @@ int sdp_get_profile_descs(const sdp_record_t *rec, sdp_list_t **profDescSeq)
 		}
 	}
 	return 0;
+fail:
+	sdp_list_free(*profDescSeq, free);
+	*profDescSeq = NULL;
+	return -1;
 }
 
 int sdp_get_server_ver(const sdp_record_t *rec, sdp_list_t **u16)
@@ -2231,7 +2267,15 @@ static sdp_data_t *access_proto_to_dataseq(sdp_record_t *rec, sdp_list_t *proto)
 
 	seqlen = sdp_list_len(proto);
 	seqDTDs = (void **)malloc(seqlen * sizeof(void *));
+	if (!seqDTDs)
+		return NULL;
+
 	seqs = (void **)malloc(seqlen * sizeof(void *));
+	if (!seqs) {
+		free(seqDTDs);
+		return NULL;
+	}
+
 	for (i = 0, p = proto; p; p = p->next, i++) {
 		sdp_list_t *elt = (sdp_list_t *)p->data;
 		sdp_data_t *s;
@@ -2350,10 +2394,19 @@ int sdp_set_lang_attr(sdp_record_t *rec, const sdp_list_t *seq)
 {
 	uint8_t uint16 = SDP_UINT16;
 	int status = 0, i = 0, seqlen = sdp_list_len(seq);
-	void **dtds = (void **)malloc(3 * seqlen * sizeof(void *));
-	void **values = (void **)malloc(3 * seqlen * sizeof(void *));
+	void **dtds, **values;
 	const sdp_list_t *p;
 
+	dtds = (void **)malloc(3 * seqlen * sizeof(void *));
+	if (!dtds)
+		return -1;
+
+	values = (void **)malloc(3 * seqlen * sizeof(void *));
+	if (!values) {
+		free(dtds);
+		return -1;
+	}
+
 	for (p = seq; p; p = p->next) {
 		sdp_lang_attr_t *lang = (sdp_lang_attr_t *)p->data;
 		if (!lang) {
@@ -2455,10 +2508,19 @@ int sdp_set_profile_descs(sdp_record_t *rec, const sdp_list_t *profiles)
 	uint8_t uuid128 = SDP_UUID128;
 	uint8_t uint16 = SDP_UINT16;
 	int i = 0, seqlen = sdp_list_len(profiles);
-	void **seqDTDs = (void **)malloc(seqlen * sizeof(void *));
-	void **seqs = (void **)malloc(seqlen * sizeof(void *));
+	void **seqDTDs, **seqs;
 	const sdp_list_t *p;
 
+	seqDTDs = (void **)malloc(seqlen * sizeof(void *));
+	if (!seqDTDs)
+		return -1;
+
+	seqs = (void **)malloc(seqlen * sizeof(void *));
+	if (!seqs) {
+		free(seqDTDs);
+		return -1;
+	}
+
 	for (p = profiles; p; p = p->next) {
 		sdp_data_t *seq;
 		void *dtds[2], *values[2];
@@ -2643,6 +2705,10 @@ void sdp_uuid32_to_uuid128(uuid_t *uuid128, uuid_t *uuid32)
 uuid_t *sdp_uuid_to_uuid128(uuid_t *uuid)
 {
 	uuid_t *uuid128 = bt_malloc(sizeof(uuid_t));
+
+	if (!uuid128)
+		return NULL;
+
 	memset(uuid128, 0, sizeof(uuid_t));
 	switch (uuid->type) {
 	case SDP_UUID128:
@@ -3087,6 +3153,10 @@ int sdp_record_update(sdp_session_t *session, const sdp_record_t *rec)
 sdp_record_t *sdp_record_alloc()
 {
 	sdp_record_t *rec = malloc(sizeof(sdp_record_t));
+
+	if (!rec)
+		return NULL;
+
 	memset((void *)rec, 0, sizeof(sdp_record_t));
 	rec->handle = 0xffffffff;
 	return rec;
-- 
1.6.3.3


^ permalink raw reply related

* Re: [PATCH 3/3] SDP
From: Santiago Carot Nemesio @ 2010-04-27 15:59 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth
In-Reply-To: <20100427150516.GA11895@jh-x301>

El mar, 27-04-2010 a las 18:05 +0300, Johan Hedberg escribió:
> Hi,
> 
> On Tue, Apr 27, 2010, Santiago Carot Nemesio wrote:
> > This patch add memory checks after malloc function is called.
> > 
> > >From 7cce3be5a1a1d506f4da71fc394ee3ccf71d8159 Mon Sep 17 00:00:00 2001
> > From: Santiago Carot Nemesio <sancane@gmail.com>
> > Date: Tue, 27 Apr 2010 15:19:44 +0200
> > Subject: [PATCH 3/3] Added memory checks
> > 
> > 
> > Signed-off-by: Santiago Carot Nemesio <sancane@gmail.com>
> 
> I've pushed the first two patches upstream but I did have to fix their
> commit messages before that. If you try "git am" yourself for what you
> sent you'll see that the summary line becomes just "SDP" for all of them
> and it should be more descriptive than that (for examples see the bluez
> commit history).
> 
> The third patch should also see these changes but additionally there
> were a few issues I noticed in the actual code:
> 
> >  			u = malloc(sizeof(uuid_t));
> > +			if (!u) {
> > +				errno = ENOMEM;
> > +				goto fail;
> > +			}
> 

I known it, but much parts in sdp.c code still set errno to ENOMEM value
when malloc fails, it is the reason which i set it to ENOMEM too.

> malloc will set the errno for you on failure so you shouldn't need to do
> it in your code.
> 
> >  			lang = malloc(sizeof(sdp_lang_attr_t));
> > +			if (!lang) {
> > +				errno = ENOMEM;
> > +				goto fail;
> 
> Same here.
> 
> > -	uuid_t *uuid128 = bt_malloc(sizeof(uuid_t));
> > +	uuid_t *uuid128 = malloc(sizeof(uuid_t));
> 
> This seems unrelated to the other changes in the patch. I don't know the
> exact philosophy of when bt_malloc should be used and when malloc should
> be used (maybe Marcel can comment on that), but I'd simply leave this
> change out of the patch.
> 
> Johan



^ permalink raw reply

* Re: [PATCH] patch to request new firmware for AR3011 Chip
From: Luis R. Rodriguez @ 2010-04-27 15:55 UTC (permalink / raw)
  To: Suraj Sumangala
  Cc: linux-bluetooth@vger.kernel.org, dwmw2@infradead.org,
	Luis Rodriguez, Jothikumar Mothilal, marcel@holtmann.org,
	Vikram Kandukuri
In-Reply-To: <1272356922.1393.14.camel@atheros013-desktop>

On Tue, Apr 27, 2010 at 01:28:42AM -0700, Suraj Sumangala wrote:
> 
> Signed-off-by: Vikram Kandukuri <vkandukuri@atheros.com>
> ---
>  drivers/bluetooth/ath3k.c |    9 ++++++---
>  1 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/bluetooth/ath3k.c b/drivers/bluetooth/ath3k.c
> index 128cae4..7bda549 100644
> --- a/drivers/bluetooth/ath3k.c
> +++ b/drivers/bluetooth/ath3k.c
> @@ -122,9 +122,12 @@ static int ath3k_probe(struct usb_interface *intf,
>  
>  	data->udev = udev;
>  
> -	if (request_firmware(&firmware, "ath3k-1.fw", &udev->dev) < 0) {
> -		kfree(data);
> -		return -EIO;
> +	if (request_firmware(&firmware, "ath3k-2.fw", &udev->dev) < 0) {
> +		BT_DBG("requesting old firmware");
> +		if (request_firmware(&firmware, "ath3k-1.fw", &udev->dev) < 0) {
> +			kfree(data);
> +			return -EIO;
> +		}
>  	}
>  
>  	size = max_t(uint, firmware->size, 4096);
> 

Looks OK but your subject should be something like:

[PATCH] ath3k: add support for new firmware

And your commit log is empty, other than your subject, please
be a little more descriptive. Describe what are the shiny new bells
and whistles added onto the new firmware. The more description
you can provide, the better.

  Luis

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox