* [PATCH 3/8] Bluetooth: Fix checking for valid device class values
From: Johan Hedberg @ 2013-01-09 13:29 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1357738180-4128-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
The two lowest bits of the minor device class value are reserved and
should be zero, and the three highest bits of the major device class
likewise. The management code should therefore test for this and return
a proper "invalid params" error if the condition is not met.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index aaf9ce6..2785de2 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -1430,6 +1430,12 @@ static int set_dev_class(struct sock *sk, struct hci_dev *hdev, void *data,
goto unlock;
}
+ if ((cp->minor & 0x03) != 0 || (cp->major & 0xe0) != 0) {
+ err = cmd_status(sk, hdev->id, MGMT_OP_SET_DEV_CLASS,
+ MGMT_STATUS_INVALID_PARAMS);
+ goto unlock;
+ }
+
hdev->major_class = cp->major;
hdev->minor_class = cp->minor;
--
1.7.10.4
^ permalink raw reply related
* [PATCH 2/8] Bluetooth: Fix missing command complete for mgmt_load_long_term_keys
From: Johan Hedberg @ 2013-01-09 13:29 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1357738180-4128-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
All management events are expected to indicate successful completion
through a command complete event, however the load long term keys
command was missing this. This patch adds the missing event.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 3bcb63e..aaf9ce6 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2665,7 +2665,7 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
{
struct mgmt_cp_load_long_term_keys *cp = cp_data;
u16 key_count, expected_len;
- int i;
+ int i, err;
key_count = __le16_to_cpu(cp->key_count);
@@ -2699,9 +2699,12 @@ static int load_long_term_keys(struct sock *sk, struct hci_dev *hdev,
key->enc_size, key->ediv, key->rand);
}
+ err = cmd_complete(sk, hdev->id, MGMT_OP_LOAD_LONG_TERM_KEYS, 0,
+ NULL, 0);
+
hci_dev_unlock(hdev);
- return 0;
+ return err;
}
static const struct mgmt_handler {
--
1.7.10.4
^ permalink raw reply related
* [PATCH 1/8] Bluetooth: Fix missing command complete event for mgmt_confirm_name
From: Johan Hedberg @ 2013-01-09 13:29 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1357738180-4128-1-git-send-email-johan.hedberg@gmail.com>
From: Johan Hedberg <johan.hedberg@intel.com>
All management commands are expected to indicate successful completion
through a command complete event however the confirm name command was
missing it. This patch add the sending of the missing event.
Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
net/bluetooth/mgmt.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index f559b96..3bcb63e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2510,7 +2510,8 @@ static int confirm_name(struct sock *sk, struct hci_dev *hdev, void *data,
hci_inquiry_cache_update_resolve(hdev, e);
}
- err = 0;
+ err = cmd_complete(sk, hdev->id, MGMT_OP_CONFIRM_NAME, 0, &cp->addr,
+ sizeof(cp->addr));
failed:
hci_dev_unlock(hdev);
--
1.7.10.4
^ permalink raw reply related
* [PATCH 0/8] Bluetooth: Various mgmt fixes
From: Johan Hedberg @ 2013-01-09 13:29 UTC (permalink / raw)
To: linux-bluetooth
Hi,
This this set of patches fixes various issues with the management
interface that have been found in the past few weeks. It also improves
the pass-rate of the user space mgmt-tester to all but one test case
(which I'm not 100% sure is correct anyway):
Total: 35, Passed: 34 (97.1%), Failed: 1, Not Run: 0
Johan Hedberg (8):
Bluetooth: Fix missing command complete event for mgmt_confirm_name
Bluetooth: Fix missing command complete for mgmt_load_long_term_keys
Bluetooth: Fix checking for valid device class values
Bluetooth: Fix accepting set_dev_class for non-BR/EDR controllers
Bluetooth: Fix returning proper command status for start_discovery
Bluetooth: Move non-critical sections outside of the dev lock
Bluetooth: Fix checking for exact values of boolean mgmt parameters
Bluetooth: Fix sending incorrect new_settings for mgmt_set_powered
net/bluetooth/mgmt.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------------
1 file changed, 91 insertions(+), 33 deletions(-)
Johan
^ permalink raw reply
* Re: [PATCH BlueZ 1/5] health: Fix pointer to local variable out-of-scope
From: Johan Hedberg @ 2013-01-09 9:15 UTC (permalink / raw)
To: Syam Sidhardhan; +Cc: linux-bluetooth
In-Reply-To: <1357645069-30841-1-git-send-email-s.syam@samsung.com>
Hi Syam,
On Tue, Jan 08, 2013, Syam Sidhardhan wrote:
> The address of the local variable is used outside the scope.
> ---
> profiles/health/hdp_util.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
All patches have been applied. Thanks. Btw, I made a minor fix to the
g_test_fail patch to use g_assert_not_reached() instead of (a less
intuitive imo) g_assert(0).
Johan
^ permalink raw reply
* Re: [PATCH] Bluetooth: btmrvl_sdio: look for sd8688 firmware in alternate place
From: Marcel Holtmann @ 2013-01-09 6:35 UTC (permalink / raw)
To: Lubomir Rintel
Cc: Bing Zhao, David Woodhouse, Ben Hutchings,
libertas-dev@lists.infradead.org, linux-bluetooth@vger.kernel.org,
Gustavo Padovan, Johan Hedberg, linux-kernel@vger.kernel.org
In-Reply-To: <1357713088.1105.1.camel@unicorn>
Hi Lubomir,
> > > > linux-firmware ships the sd8688* firmware images that are shared with
> > > > libertas_sdio WiFi driver under libertas/. libertas_sdio looks in both places
> > > > and so should we.
> > > >
> > > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > > > ---
> > > > drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++--
> > > > drivers/bluetooth/btmrvl_sdio.h | 6 ++++--
> > > > 2 files changed, 26 insertions(+), 4 deletions(-)
> > >
> > > NAK from me on this one. I do not want the driver to check two
> > > locations. That is what userspace can work around.
> > >
> > > If we want to unify the location between the WiFi driver and the
> > > Bluetooth driver, I am fine with that, but seriously, just pick one over
> > > the other. I do not care which one.
> >
> > The unified location is mrvl/ directory.
> >
> > We can probably move SD8688 firmware & helper binaries to mrvl/ and have both drivers grab the images there?
>
> That would break existing setups, wouldn't it?
>
> I was under impression (commit 3d32a58b) that we care about
> compatibility here. Do we?
that is what symlinks are for.
Regards
Marcel
^ permalink raw reply
* RE: [PATCH] Bluetooth: btmrvl_sdio: look for sd8688 firmware in alternate place
From: Lubomir Rintel @ 2013-01-09 6:31 UTC (permalink / raw)
To: Bing Zhao
Cc: Marcel Holtmann, David Woodhouse, Ben Hutchings,
libertas-dev@lists.infradead.org, linux-bluetooth@vger.kernel.org,
Gustavo Padovan, Johan Hedberg, linux-kernel@vger.kernel.org
In-Reply-To: <477F20668A386D41ADCC57781B1F70430D13FDB783@SC-VEXCH1.marvell.com>
On Tue, 2013-01-08 at 18:43 -0800, Bing Zhao wrote:
> > > linux-firmware ships the sd8688* firmware images that are shared with
> > > libertas_sdio WiFi driver under libertas/. libertas_sdio looks in both places
> > > and so should we.
> > >
> > > Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> > > ---
> > > drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++--
> > > drivers/bluetooth/btmrvl_sdio.h | 6 ++++--
> > > 2 files changed, 26 insertions(+), 4 deletions(-)
> >
> > NAK from me on this one. I do not want the driver to check two
> > locations. That is what userspace can work around.
> >
> > If we want to unify the location between the WiFi driver and the
> > Bluetooth driver, I am fine with that, but seriously, just pick one over
> > the other. I do not care which one.
>
> The unified location is mrvl/ directory.
>
> We can probably move SD8688 firmware & helper binaries to mrvl/ and have both drivers grab the images there?
That would break existing setups, wouldn't it?
I was under impression (commit 3d32a58b) that we care about
compatibility here. Do we?
--
Lubomir Rintel
^ permalink raw reply
* RE: [PATCH] Bluetooth: btmrvl_sdio: look for sd8688 firmware in alternate place
From: Bing Zhao @ 2013-01-09 2:43 UTC (permalink / raw)
To: Marcel Holtmann, Lubomir Rintel
Cc: David Woodhouse, Ben Hutchings, libertas-dev@lists.infradead.org,
linux-bluetooth@vger.kernel.org, Gustavo Padovan, Johan Hedberg,
linux-kernel@vger.kernel.org
In-Reply-To: <1357698538.1806.43.camel@aeonflux>
DQo+ID4gbGludXgtZmlybXdhcmUgc2hpcHMgdGhlIHNkODY4OCogZmlybXdhcmUgaW1hZ2VzIHRo
YXQgYXJlIHNoYXJlZCB3aXRoDQo+ID4gbGliZXJ0YXNfc2RpbyBXaUZpIGRyaXZlciB1bmRlciBs
aWJlcnRhcy8uIGxpYmVydGFzX3NkaW8gbG9va3MgaW4gYm90aCBwbGFjZXMNCj4gPiBhbmQgc28g
c2hvdWxkIHdlLg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogTHVib21pciBSaW50ZWwgPGxrdW5k
cmFrQHYzLnNrPg0KPiA+IC0tLQ0KPiA+ICBkcml2ZXJzL2JsdWV0b290aC9idG1ydmxfc2Rpby5j
IHwgICAyNCArKysrKysrKysrKysrKysrKysrKysrLS0NCj4gPiAgZHJpdmVycy9ibHVldG9vdGgv
YnRtcnZsX3NkaW8uaCB8ICAgIDYgKysrKy0tDQo+ID4gIDIgZmlsZXMgY2hhbmdlZCwgMjYgaW5z
ZXJ0aW9ucygrKSwgNCBkZWxldGlvbnMoLSkNCj4gDQo+IE5BSyBmcm9tIG1lIG9uIHRoaXMgb25l
LiBJIGRvIG5vdCB3YW50IHRoZSBkcml2ZXIgdG8gY2hlY2sgdHdvDQo+IGxvY2F0aW9ucy4gVGhh
dCBpcyB3aGF0IHVzZXJzcGFjZSBjYW4gd29yayBhcm91bmQuDQo+IA0KPiBJZiB3ZSB3YW50IHRv
IHVuaWZ5IHRoZSBsb2NhdGlvbiBiZXR3ZWVuIHRoZSBXaUZpIGRyaXZlciBhbmQgdGhlDQo+IEJs
dWV0b290aCBkcml2ZXIsIEkgYW0gZmluZSB3aXRoIHRoYXQsIGJ1dCBzZXJpb3VzbHksIGp1c3Qg
cGljayBvbmUgb3Zlcg0KPiB0aGUgb3RoZXIuIEkgZG8gbm90IGNhcmUgd2hpY2ggb25lLg0KDQpU
aGUgdW5pZmllZCBsb2NhdGlvbiBpcyBtcnZsLyBkaXJlY3RvcnkuDQoNCldlIGNhbiBwcm9iYWJs
eSBtb3ZlIFNEODY4OCBmaXJtd2FyZSAmIGhlbHBlciBiaW5hcmllcyB0byBtcnZsLyBhbmQgaGF2
ZSBib3RoIGRyaXZlcnMgZ3JhYiB0aGUgaW1hZ2VzIHRoZXJlPw0KDQpSZWdhcmRzLA0KQmluZw0K
DQo=
^ permalink raw reply
* Re: [PATCH] Bluetooth: btmrvl_sdio: look for sd8688 firmware in alternate place
From: Marcel Holtmann @ 2013-01-09 2:28 UTC (permalink / raw)
To: Lubomir Rintel
Cc: David Woodhouse, Ben Hutchings, libertas-dev, linux-bluetooth,
Gustavo Padovan, Johan Hedberg, linux-kernel
In-Reply-To: <1357689361-7969-2-git-send-email-lkundrak@v3.sk>
Hi Lubomir,
> linux-firmware ships the sd8688* firmware images that are shared with
> libertas_sdio WiFi driver under libertas/. libertas_sdio looks in both places
> and so should we.
>
> Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
> ---
> drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++--
> drivers/bluetooth/btmrvl_sdio.h | 6 ++++--
> 2 files changed, 26 insertions(+), 4 deletions(-)
NAK from me on this one. I do not want the driver to check two
locations. That is what userspace can work around.
If we want to unify the location between the WiFi driver and the
Bluetooth driver, I am fine with that, but seriously, just pick one over
the other. I do not care which one.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH v2 BlueZ] lib: Fix SDP_TEXT_STR16/SDP_URL_STR16 parsing
From: Marcel Holtmann @ 2013-01-09 2:25 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1357686325-29529-1-git-send-email-anderson.lizardo@openbossa.org>
Hi Anderson,
> sdp_extract_attr() uses the "size" parameter to return the number of
> bytes consumed when parsing SDP Data Elements. This size is used to
> advance a buffer pointer to parse next element.
>
> This size was being incorrectly calculated for SDP_{TEXT,URL}_STR16 in
> extract_str(), where the string length was added twice. The string
> length is already added later in the function for {TEXT,URL}_STR{8,16}
> by this statement:
>
> *len += n;
> ---
> lib/sdp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
patch has been applied.
Regards
Marcel
^ permalink raw reply
* [PATCH] Move sd8688*.bin images away from libertas tree
From: Lubomir Rintel @ 2013-01-08 23:57 UTC (permalink / raw)
To: David Woodhouse, Ben Hutchings, libertas-dev, linux-bluetooth
Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, linux-kernel,
lkundrak
In-Reply-To: <1357689361-7969-1-git-send-email-lkundrak@v3.sk>
They are (unlike rest of sd8xxx images from libertas/ and mrvl/) not Libertas
WiFi specific and are used for the bluetooth controller (btmrvl) which does not
look for them in libertas/.
This is fine for the WiFi driver that utilizes those, since it has always been
looking in this place as well.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
libertas/sd8688.bin => sd8688.bin | Bin 259172 -> 259172 bytes
libertas/sd8688_helper.bin => sd8688_helper.bin | Bin 2616 -> 2616 bytes
2 files changed, 0 insertions(+), 0 deletions(-)
rename libertas/sd8688.bin => sd8688.bin (100%)
rename libertas/sd8688_helper.bin => sd8688_helper.bin (100%)
--
1.7.1
^ permalink raw reply
* [PATCH] Bluetooth: btmrvl_sdio: look for sd8688 firmware in alternate place
From: Lubomir Rintel @ 2013-01-08 23:56 UTC (permalink / raw)
To: David Woodhouse, Ben Hutchings, libertas-dev, linux-bluetooth
Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, linux-kernel,
lkundrak
In-Reply-To: <1357689361-7969-1-git-send-email-lkundrak@v3.sk>
linux-firmware ships the sd8688* firmware images that are shared with
libertas_sdio WiFi driver under libertas/. libertas_sdio looks in both places
and so should we.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
drivers/bluetooth/btmrvl_sdio.c | 24 ++++++++++++++++++++++--
drivers/bluetooth/btmrvl_sdio.h | 6 ++++--
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/bluetooth/btmrvl_sdio.c b/drivers/bluetooth/btmrvl_sdio.c
index 9959d4c..494f921 100644
--- a/drivers/bluetooth/btmrvl_sdio.c
+++ b/drivers/bluetooth/btmrvl_sdio.c
@@ -83,22 +83,28 @@ static const struct btmrvl_sdio_card_reg btmrvl_reg_87xx = {
};
static const struct btmrvl_sdio_device btmrvl_sdio_sd8688 = {
- .helper = "sd8688_helper.bin",
- .firmware = "sd8688.bin",
+ .helper = "libertas/sd8688_helper.bin",
+ .helper2 = "sd8688_helper.bin",
+ .firmware = "libertas/sd8688.bin",
+ .firmware2 = "sd8688.bin",
.reg = &btmrvl_reg_8688,
.sd_blksz_fw_dl = 64,
};
static const struct btmrvl_sdio_device btmrvl_sdio_sd8787 = {
.helper = NULL,
+ .helper2 = NULL,
.firmware = "mrvl/sd8787_uapsta.bin",
+ .firmware2 = NULL,
.reg = &btmrvl_reg_87xx,
.sd_blksz_fw_dl = 256,
};
static const struct btmrvl_sdio_device btmrvl_sdio_sd8797 = {
.helper = NULL,
+ .helper2 = NULL,
.firmware = "mrvl/sd8797_uapsta.bin",
+ .firmware2 = NULL,
.reg = &btmrvl_reg_87xx,
.sd_blksz_fw_dl = 256,
};
@@ -260,6 +266,11 @@ static int btmrvl_sdio_download_helper(struct btmrvl_sdio_card *card)
ret = request_firmware(&fw_helper, card->helper,
&card->func->dev);
+ if (ret < 0 && card->helper2) {
+ release_firmware(fw_helper);
+ ret = request_firmware(&fw_helper, card->helper2,
+ &card->func->dev);
+ }
if ((ret < 0) || !fw_helper) {
BT_ERR("request_firmware(helper) failed, error code = %d",
ret);
@@ -360,6 +371,11 @@ static int btmrvl_sdio_download_fw_w_helper(struct btmrvl_sdio_card *card)
ret = request_firmware(&fw_firmware, card->firmware,
&card->func->dev);
+ if (ret < 0 && card->firmware2) {
+ release_firmware(fw_firmware);
+ ret = request_firmware(&fw_firmware, card->firmware2,
+ &card->func->dev);
+ }
if ((ret < 0) || !fw_firmware) {
BT_ERR("request_firmware(firmware) failed, error code = %d",
ret);
@@ -970,7 +986,9 @@ static int btmrvl_sdio_probe(struct sdio_func *func,
if (id->driver_data) {
struct btmrvl_sdio_device *data = (void *) id->driver_data;
card->helper = data->helper;
+ card->helper2 = data->helper2;
card->firmware = data->firmware;
+ card->firmware2 = data->firmware2;
card->reg = data->reg;
card->sd_blksz_fw_dl = data->sd_blksz_fw_dl;
}
@@ -1186,6 +1204,8 @@ MODULE_DESCRIPTION("Marvell BT-over-SDIO driver ver " VERSION);
MODULE_VERSION(VERSION);
MODULE_LICENSE("GPL v2");
MODULE_FIRMWARE("sd8688_helper.bin");
+MODULE_FIRMWARE("libertas/sd8688_helper.bin");
MODULE_FIRMWARE("sd8688.bin");
+MODULE_FIRMWARE("libertas/sd8688.bin");
MODULE_FIRMWARE("mrvl/sd8787_uapsta.bin");
MODULE_FIRMWARE("mrvl/sd8797_uapsta.bin");
diff --git a/drivers/bluetooth/btmrvl_sdio.h b/drivers/bluetooth/btmrvl_sdio.h
index 43d35a6..4a5a019 100644
--- a/drivers/bluetooth/btmrvl_sdio.h
+++ b/drivers/bluetooth/btmrvl_sdio.h
@@ -84,7 +84,9 @@ struct btmrvl_sdio_card {
struct sdio_func *func;
u32 ioport;
const char *helper;
+ const char *helper2;
const char *firmware;
+ const char *firmware2;
const struct btmrvl_sdio_card_reg *reg;
u16 sd_blksz_fw_dl;
u8 rx_unit;
@@ -92,8 +94,8 @@ struct btmrvl_sdio_card {
};
struct btmrvl_sdio_device {
- const char *helper;
- const char *firmware;
+ const char *helper, *helper2;
+ const char *firmware, *firmware2;
const struct btmrvl_sdio_card_reg *reg;
u16 sd_blksz_fw_dl;
};
--
1.7.1
^ permalink raw reply related
* sd8688 firmware location
From: Lubomir Rintel @ 2013-01-08 23:56 UTC (permalink / raw)
To: David Woodhouse, Ben Hutchings, libertas-dev, linux-bluetooth
Cc: Marcel Holtmann, Gustavo Padovan, Johan Hedberg, linux-kernel,
lkundrak
Hi!
btmrvl_sdio and libertas_sdio both use firmware files sd8688.bin and
sd8688_helper.bin. In linux-firmware, they're present in libertas/ tree and
(since 3d32a58b) libertas_sdio perfers loading it from there, while it is able
to fallback to load it from linux-firmware root. btmrvl_sdio, on the other hand
only looks in the root and ends up not being successful.
Obviously, there are two solutions to the problem -- either teach btmrvl_sdio
to look into libertas/, or move the files in linux-firmware tree. I don't
really have a strong preference, though it probably makes less sense to keep in
in libertas/, since the bluetooth hardware is not really marketed as "Libertas."
I'm following up with patches to linux and linux-firmware and I'd be very
thankful if you could pick one (not both of them).
Have a nice day!
--
Lubomir Rintel >o
(\)
^ permalink raw reply
* [PATCH v2 BlueZ] lib: Fix SDP_TEXT_STR16/SDP_URL_STR16 parsing
From: Anderson Lizardo @ 2013-01-08 23:05 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1357670608-19081-2-git-send-email-anderson.lizardo@openbossa.org>
sdp_extract_attr() uses the "size" parameter to return the number of
bytes consumed when parsing SDP Data Elements. This size is used to
advance a buffer pointer to parse next element.
This size was being incorrectly calculated for SDP_{TEXT,URL}_STR16 in
extract_str(), where the string length was added twice. The string
length is already added later in the function for {TEXT,URL}_STR{8,16}
by this statement:
*len += n;
---
lib/sdp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index ca474cd..b87f392 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1176,7 +1176,7 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
}
n = bt_get_be16(p);
p += sizeof(uint16_t);
- *len += sizeof(uint16_t) + n;
+ *len += sizeof(uint16_t);
bufsize -= sizeof(uint16_t);
break;
default:
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH BlueZ 1/2] unit: Add initial tests for sdp_extract_attr()
From: Marcel Holtmann @ 2013-01-08 22:46 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <CAJdJm_Nij11_3FdUaCD7s4Z6B3wnncVrKHg8YaF2wHHbEd1-1g@mail.gmail.com>
Hi Anderson,
> > can we make this a bit more generic with a bit more details on what you
> > are testing.
> >
> > Also having a separate test case for str8, str16 and also str32 of
> > course would be a good idea. Same for url8, url16 and url32. In addition
> > checking empty strings and really long strings is a good idea.
> > Especially long strings that match the max len size.
>
> This was supposed to be a set of initial tests, specially to validate
> the fix I sent on the other patch. I was going to improve the test
> coverage as I read more of the code.
>
> But I can work on a set of tests which cover all reachable cases for
> sdp_extract_attr() (I want to focus on this function for now because
> it is less used than others and could hide other bugs.) and send them
> at once, including corner cases.
I want to make our life easier in the long term. Otherwise you end up
with copy&paste all over the place.
> > What I actually like to see is that we can specific element sequences in
> > raw and also what they are suppose to match. So we need to ensure that
> > we also extract the right string value and types. And not just the size.
>
> The "match" data could be raw bytes which I get by converting the
> returned sdp_data_t to PDU format using sdp_gen_pdu(). What do you
> think?
A better test case data could look like this:
struct test_data {
const void *input_data;
size_t input_size;
uint8_t dtd;
};
And we can add combinations of structs and unions here to represent the
different testes.
Also I would create a nice helper define like I did with define_ssa()
for example to create the test data and the test with a nice name.
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ 2/2] lib: Fix SDP_TEXT_STR16/SDP_URL_STR16 parsing
From: Marcel Holtmann @ 2013-01-08 22:41 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <CAJdJm_PV7qMJWQLJBOmA0tqqDzor6immypKGgGTtLVy9SR5A8A@mail.gmail.com>
Hi Anderson,
> >> diff --git a/lib/sdp.c b/lib/sdp.c
> >> index ca474cd..b87f392 100644
> >> --- a/lib/sdp.c
> >> +++ b/lib/sdp.c
> >> @@ -1176,7 +1176,7 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
> >> }
> >> n = bt_get_be16(p);
> >> p += sizeof(uint16_t);
> >> - *len += sizeof(uint16_t) + n;
> >> + *len += sizeof(uint16_t);
> >
> > I do not get this fix. Isn't this str8 and url8 part wrong?
>
> On the same function, next to the return, there is:
>
> *len += n;
>
> This is why STR8/URL8 actually are okay. For STR16 the "n" part is
> added twice due to the snippet I change in this patch.
good catch. However I like to see that tiny piece of extra information
in the commit message as well. Makes it a lot clearer.
Regards
Marcel
^ permalink raw reply
* Bluetooth / TTY: [ 1806.484970] INFO: task kworker/0:1:25023 blocked for more than 120 seconds.
From: Sander Eikelenboom @ 2013-01-08 22:00 UTC (permalink / raw)
To: linux-kernel, linux-bluetooth, linux-serial
Cc: marcel, Greg Kroah-Hartman, Alan Cox
I'm trying to use a USB bluetooth dongle to connect to a bluetooth to serial device with RFCOMM.
It's able to work fine for some time, but tt consistently fails after some time.
This is sometimes right on the start when connecting to the /dev/rfcomm0, but it can also require several hours of running fine while connected and exchanging data.
This is the stacktrace i get:
[ 1806.484970] INFO: task kworker/0:1:25023 blocked for more than 120 seconds.
[ 1806.503488] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 1806.521864] kworker/0:1 D 0000000000000201 0 25023 2 0x00000000
[ 1806.540026] ffff88000baa7be8 0000000000000216 ffff880037079148 ffff880037079148
[ 1806.557926] ffff8800386fa0e0 0000000000013040 ffff88000baa7fd8 ffff88000baa6010
[ 1806.575622] 0000000000013040 0000000000013040 ffff88000baa7fd8 0000000000013040
[ 1806.592981] Call Trace:
[ 1806.610066] [<ffffffff810b5bd7>] ? lock_release+0x117/0x250
[ 1806.627150] [<ffffffff810b5748>] ? lock_acquire+0xd8/0x100
[ 1806.643901] [<ffffffff819ba2fe>] ? tty_lock_nested+0x3e/0x80
[ 1806.660460] [<ffffffff819b8a14>] schedule+0x24/0x70
[ 1806.676724] [<ffffffff819b8ef3>] schedule_preempt_disabled+0x13/0x20
[ 1806.692780] [<ffffffff819b73bb>] mutex_lock_nested+0x1ab/0x450
[ 1806.708582] [<ffffffff819ba2fe>] ? tty_lock_nested+0x3e/0x80
[ 1806.724140] [<ffffffff819ba2fe>] tty_lock_nested+0x3e/0x80
[ 1806.739421] [<ffffffff819ba34b>] tty_lock+0xb/0x10
[ 1806.754418] [<ffffffff81449495>] __tty_hangup+0x65/0x3c0
[ 1806.769153] [<ffffffff81080bf8>] ? process_one_work+0x158/0x4b0
[ 1806.783648] [<ffffffff81449800>] do_tty_hangup+0x10/0x20
[ 1806.797905] [<ffffffff81080c60>] process_one_work+0x1c0/0x4b0
[ 1806.811958] [<ffffffff81080bf8>] ? process_one_work+0x158/0x4b0
[ 1806.825752] [<ffffffff814497f0>] ? __tty_hangup+0x3c0/0x3c0
[ 1806.839332] [<ffffffff8108134e>] worker_thread+0x11e/0x3d0
[ 1806.852654] [<ffffffff81081230>] ? manage_workers+0x2e0/0x2e0
[ 1806.865719] [<ffffffff81088a36>] kthread+0xd6/0xe0
[ 1806.878518] [<ffffffff81088960>] ? __init_kthread_worker+0x70/0x70
[ 1806.891064] [<ffffffff819baebc>] ret_from_fork+0x7c/0xb0
[ 1806.903376] [<ffffffff81088960>] ? __init_kthread_worker+0x70/0x70
[ 1806.939888] INFO: lockdep is turned off.
[ 1806.951766] INFO: task zabbix_slimmeme:27798 blocked for more than 120 seconds.
[ 1806.963521] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
[ 1806.975059] zabbix_slimmeme D ffff88002a619070 0 27798 27355 0x00000000
[ 1806.986497] ffff880000bb7818 0000000000000216 ffff880000000002 ffffffff8202ae38
[ 1806.997893] ffff88002a619070 0000000000013040 ffff880000bb7fd8 ffff880000bb6010
[ 1807.008944] 0000000000013040 0000000000013040 ffff880000bb7fd8 0000000000013040
[ 1807.019692] Call Trace:
[ 1807.030165] [<ffffffff810be1ad>] ? __module_text_address+0xd/0x60
[ 1807.040524] [<ffffffff810be1ad>] ? __module_text_address+0xd/0x60
[ 1807.050568] [<ffffffff810be40b>] ? is_module_text_address+0x2b/0x60
[ 1807.060389] [<ffffffff81085958>] ? __kernel_text_address+0x58/0x80
[ 1807.069996] [<ffffffff81070087>] ? local_bh_disable+0x17/0x20
[ 1807.079383] [<ffffffff810b5748>] ? lock_acquire+0xd8/0x100
[ 1807.088467] [<ffffffff819b8a14>] schedule+0x24/0x70
[ 1807.097296] [<ffffffff819b5c7d>] schedule_timeout+0x1bd/0x220
[ 1807.105884] [<ffffffff810b5748>] ? lock_acquire+0xd8/0x100
[ 1807.114211] [<ffffffff819b7f11>] ? wait_for_common+0x31/0x170
[ 1807.122301] [<ffffffff810b5bd7>] ? lock_release+0x117/0x250
[ 1807.130156] [<ffffffff819b7fe1>] wait_for_common+0x101/0x170
[ 1807.137804] [<ffffffff810986f0>] ? try_to_wake_up+0x310/0x310
[ 1807.145193] [<ffffffff819b80f8>] wait_for_completion+0x18/0x20
[ 1807.152350] [<ffffffff81083385>] flush_work+0x195/0x250
[ 1807.159275] [<ffffffff810833a0>] ? flush_work+0x1b0/0x250
[ 1807.165957] [<ffffffff81080400>] ? cwq_dec_nr_in_flight+0xd0/0xd0
[ 1807.172401] [<ffffffff81451748>] tty_ldisc_flush_works+0x18/0x40
[ 1807.178634] [<ffffffff8145198e>] tty_ldisc_release+0x2e/0x90
[ 1807.184586] [<ffffffff8144ba07>] tty_release+0x3c7/0x590
[ 1807.190264] [<ffffffff810b19ed>] ? trace_hardirqs_on+0xd/0x10
[ 1807.195910] [<ffffffff819b60b9>] ? __mutex_unlock_slowpath+0x149/0x1d0
[ 1807.201455] [<ffffffff810986f0>] ? try_to_wake_up+0x310/0x310
[ 1807.206927] [<ffffffff8144bf94>] tty_open+0x3c4/0x5f0
[ 1807.212366] [<ffffffff81150c88>] chrdev_open+0x98/0x170
[ 1807.217803] [<ffffffff8109128d>] ? lg_local_unlock+0x3d/0x70
[ 1807.223255] [<ffffffff81150bf0>] ? cdev_put+0x30/0x30
[ 1807.228678] [<ffffffff8114b46e>] do_dentry_open+0x25e/0x310
[ 1807.234040] [<ffffffff8114b630>] finish_open+0x30/0x50
[ 1807.239445] [<ffffffff8115aa0e>] do_last+0x30e/0xe90
[ 1807.244805] [<ffffffff81157d2a>] ? link_path_walk+0x9a/0x9f0
[ 1807.250170] [<ffffffff8115b63e>] path_openat+0xae/0x4e0
[ 1807.255503] [<ffffffff810b5bd7>] ? lock_release+0x117/0x250
[ 1807.260835] [<ffffffff811602d4>] ? do_select+0x3f4/0x6d0
[ 1807.266174] [<ffffffff8115bba4>] do_filp_open+0x44/0xa0
[ 1807.271504] [<ffffffff81169453>] ? __alloc_fd+0xb3/0x150
[ 1807.276904] [<ffffffff8114af83>] do_sys_open+0x103/0x1f0
[ 1807.282262] [<ffffffff8114b0ac>] sys_open+0x1c/0x20
[ 1807.287579] [<ffffffff819baf69>] system_call_fastpath+0x16/0x1b
[ 1807.292892] INFO: lockdep is turned off.
^ permalink raw reply
* Re: [PATCH BlueZ 1/2] unit: Add initial tests for sdp_extract_attr()
From: Anderson Lizardo @ 2013-01-08 20:45 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1357672215.1806.32.camel@aeonflux>
Hi Marcel,
On Tue, Jan 8, 2013 at 3:10 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
> can we make this a bit more generic with a bit more details on what you
> are testing.
>
> Also having a separate test case for str8, str16 and also str32 of
> course would be a good idea. Same for url8, url16 and url32. In addition
> checking empty strings and really long strings is a good idea.
> Especially long strings that match the max len size.
This was supposed to be a set of initial tests, specially to validate
the fix I sent on the other patch. I was going to improve the test
coverage as I read more of the code.
But I can work on a set of tests which cover all reachable cases for
sdp_extract_attr() (I want to focus on this function for now because
it is less used than others and could hide other bugs.) and send them
at once, including corner cases.
> What I actually like to see is that we can specific element sequences in
> raw and also what they are suppose to match. So we need to ensure that
> we also extract the right string value and types. And not just the size.
The "match" data could be raw bytes which I get by converting the
returned sdp_data_t to PDU format using sdp_gen_pdu(). What do you
think?
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH BlueZ 2/2] lib: Fix SDP_TEXT_STR16/SDP_URL_STR16 parsing
From: Anderson Lizardo @ 2013-01-08 20:27 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
In-Reply-To: <1357672384.1806.34.camel@aeonflux>
Hi Marcel,
On Tue, Jan 8, 2013 at 3:13 PM, Marcel Holtmann <marcel@holtmann.org> wrote:
>> diff --git a/lib/sdp.c b/lib/sdp.c
>> index ca474cd..b87f392 100644
>> --- a/lib/sdp.c
>> +++ b/lib/sdp.c
>> @@ -1176,7 +1176,7 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
>> }
>> n = bt_get_be16(p);
>> p += sizeof(uint16_t);
>> - *len += sizeof(uint16_t) + n;
>> + *len += sizeof(uint16_t);
>
> I do not get this fix. Isn't this str8 and url8 part wrong?
On the same function, next to the return, there is:
*len += n;
This is why STR8/URL8 actually are okay. For STR16 the "n" part is
added twice due to the snippet I change in this patch.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH BlueZ 2/2] lib: Fix SDP_TEXT_STR16/SDP_URL_STR16 parsing
From: Marcel Holtmann @ 2013-01-08 19:13 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1357670608-19081-2-git-send-email-anderson.lizardo@openbossa.org>
Hi Anderson,
> sdp_extract_attr() uses the "size" parameter to return the number of
> bytes consumed when parsing SDP Data Elements. This size is used to
> advance a buffer pointer to parse next element.
>
> This size was being incorrectly calculated for
> SDP_TEXT_STR16/SDP_URL_STR16, where the string length was added twice.
>
> A unit test added on the previous commit should now pass with this fix.
> ---
> lib/sdp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/sdp.c b/lib/sdp.c
> index ca474cd..b87f392 100644
> --- a/lib/sdp.c
> +++ b/lib/sdp.c
> @@ -1176,7 +1176,7 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
> }
> n = bt_get_be16(p);
> p += sizeof(uint16_t);
> - *len += sizeof(uint16_t) + n;
> + *len += sizeof(uint16_t);
I do not get this fix. Isn't this str8 and url8 part wrong?
Regards
Marcel
^ permalink raw reply
* Re: [PATCH BlueZ 1/2] unit: Add initial tests for sdp_extract_attr()
From: Marcel Holtmann @ 2013-01-08 19:10 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <1357670608-19081-1-git-send-email-anderson.lizardo@openbossa.org>
Hi Anderson,
> These tests do not use the full SDP PDU building code because they try
> to catch errors on SDP "extraction" code, which may not appear on a
> response PDU (but still cause hard to find bugs).
> ---
> unit/test-sdp.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/unit/test-sdp.c b/unit/test-sdp.c
> index 315a5cd..61449aa 100644
> --- a/unit/test-sdp.c
> +++ b/unit/test-sdp.c
> @@ -754,6 +754,31 @@ static void test_sdp(gconstpointer data)
> g_free(test->pdu_list);
> }
>
> +static void test_sdp_extract_attr(void)
> +{
> + const struct sdp_pdu pdus[] = {
> + raw_pdu(SDP_DATA_NIL),
> + raw_pdu(SDP_TEXT_STR8, 0x04, 'A', 'B', 'C', 'D'),
> + raw_pdu(SDP_TEXT_STR16, 0x00, 0x04, 'A', 'B', 'C', 'D'),
> + { },
> + };
> + int i;
> +
> + for (i = 0; pdus[i].valid; i++) {
> + sdp_data_t *d;
> + int size = 0;
> +
> + if (g_test_verbose() == TRUE)
> + g_print("dtd=0x%02x\n", *(char *) pdus[i].raw_data);
> +
> + d = sdp_extract_attr(pdus[i].raw_data, pdus[i].raw_size, &size,
> + NULL);
> + g_assert(d != NULL);
> + g_assert_cmpuint(size, ==, pdus[i].raw_size);
> + sdp_data_free(d);
> + }
> +}
> +
> int main(int argc, char *argv[])
> {
> g_test_init(&argc, &argv, NULL);
> @@ -2709,5 +2734,7 @@ int main(int argc, char *argv[])
> 0x08, 0x09, 0x00, 0x01, 0x35, 0x03, 0x19, 0x11,
> 0x06, 0x00));
>
> + g_test_add_func("/MISC/sdp_extract_attr", test_sdp_extract_attr);
> +
can we make this a bit more generic with a bit more details on what you
are testing.
Also having a separate test case for str8, str16 and also str32 of
course would be a good idea. Same for url8, url16 and url32. In addition
checking empty strings and really long strings is a good idea.
Especially long strings that match the max len size.
What I actually like to see is that we can specific element sequences in
raw and also what they are suppose to match. So we need to ensure that
we also extract the right string value and types. And not just the size.
Regards
Marcel
^ permalink raw reply
* [PATCH BlueZ 2/2] lib: Fix SDP_TEXT_STR16/SDP_URL_STR16 parsing
From: Anderson Lizardo @ 2013-01-08 18:43 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1357670608-19081-1-git-send-email-anderson.lizardo@openbossa.org>
sdp_extract_attr() uses the "size" parameter to return the number of
bytes consumed when parsing SDP Data Elements. This size is used to
advance a buffer pointer to parse next element.
This size was being incorrectly calculated for
SDP_TEXT_STR16/SDP_URL_STR16, where the string length was added twice.
A unit test added on the previous commit should now pass with this fix.
---
lib/sdp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index ca474cd..b87f392 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1176,7 +1176,7 @@ static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
}
n = bt_get_be16(p);
p += sizeof(uint16_t);
- *len += sizeof(uint16_t) + n;
+ *len += sizeof(uint16_t);
bufsize -= sizeof(uint16_t);
break;
default:
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 1/2] unit: Add initial tests for sdp_extract_attr()
From: Anderson Lizardo @ 2013-01-08 18:43 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
These tests do not use the full SDP PDU building code because they try
to catch errors on SDP "extraction" code, which may not appear on a
response PDU (but still cause hard to find bugs).
---
unit/test-sdp.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/unit/test-sdp.c b/unit/test-sdp.c
index 315a5cd..61449aa 100644
--- a/unit/test-sdp.c
+++ b/unit/test-sdp.c
@@ -754,6 +754,31 @@ static void test_sdp(gconstpointer data)
g_free(test->pdu_list);
}
+static void test_sdp_extract_attr(void)
+{
+ const struct sdp_pdu pdus[] = {
+ raw_pdu(SDP_DATA_NIL),
+ raw_pdu(SDP_TEXT_STR8, 0x04, 'A', 'B', 'C', 'D'),
+ raw_pdu(SDP_TEXT_STR16, 0x00, 0x04, 'A', 'B', 'C', 'D'),
+ { },
+ };
+ int i;
+
+ for (i = 0; pdus[i].valid; i++) {
+ sdp_data_t *d;
+ int size = 0;
+
+ if (g_test_verbose() == TRUE)
+ g_print("dtd=0x%02x\n", *(char *) pdus[i].raw_data);
+
+ d = sdp_extract_attr(pdus[i].raw_data, pdus[i].raw_size, &size,
+ NULL);
+ g_assert(d != NULL);
+ g_assert_cmpuint(size, ==, pdus[i].raw_size);
+ sdp_data_free(d);
+ }
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -2709,5 +2734,7 @@ int main(int argc, char *argv[])
0x08, 0x09, 0x00, 0x01, 0x35, 0x03, 0x19, 0x11,
0x06, 0x00));
+ g_test_add_func("/MISC/sdp_extract_attr", test_sdp_extract_attr);
+
return g_test_run();
}
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 2/2] attrib: Fix compilation errors when compiled without optimization
From: Anderson Lizardo @ 2013-01-08 15:46 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
In-Reply-To: <1357659987-4219-1-git-send-email-anderson.lizardo@openbossa.org>
Fix these build errors:
attrib/att.c: In function ‘dec_read_by_grp_req’:
attrib/att.c:165:10: error: comparison between signed and unsigned
integer expressions [-Werror=sign-compare]
attrib/att.c:170:10: error: comparison between signed and unsigned
integer expressions [-Werror=sign-compare]
attrib/att.c: In function ‘dec_read_by_type_req’:
attrib/att.c:393:10: error: comparison between signed and unsigned
integer expressions [-Werror=sign-compare]
attrib/att.c:402:10: error: comparison between signed and unsigned
integer expressions [-Werror=sign-compare]
---
attrib/att.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/attrib/att.c b/attrib/att.c
index de11811..fe66821 100644
--- a/attrib/att.c
+++ b/attrib/att.c
@@ -151,7 +151,7 @@ uint16_t enc_read_by_grp_req(uint16_t start, uint16_t end, bt_uuid_t *uuid,
uint16_t dec_read_by_grp_req(const uint8_t *pdu, size_t len, uint16_t *start,
uint16_t *end, bt_uuid_t *uuid)
{
- const uint16_t min_len = sizeof(pdu[0]) + sizeof(*start) + sizeof(*end);
+ const size_t min_len = sizeof(pdu[0]) + sizeof(*start) + sizeof(*end);
if (pdu == NULL)
return 0;
@@ -382,7 +382,7 @@ uint16_t enc_read_by_type_req(uint16_t start, uint16_t end, bt_uuid_t *uuid,
uint16_t dec_read_by_type_req(const uint8_t *pdu, size_t len, uint16_t *start,
uint16_t *end, bt_uuid_t *uuid)
{
- const uint16_t min_len = sizeof(pdu[0]) + sizeof(*start) + sizeof(*end);
+ const size_t min_len = sizeof(pdu[0]) + sizeof(*start) + sizeof(*end);
if (pdu == NULL)
return 0;
--
1.7.9.5
^ permalink raw reply related
* [PATCH BlueZ 1/2] build: Fix --disable-optimization configure option
From: Anderson Lizardo @ 2013-01-08 15:46 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Anderson Lizardo
On commit cc9e4e7cae0379864ea06038d92bf7ecc192bba7, this flag was
mistakenly replaced with the behavior of the old --enable-fortify
option.
This patch restores the "-O0" flag when --disable-optimization is used.
Unfortunately, this is not enough to disable build optimization. By
default, autoconf adds -O2 to CFLAGS if the compiler is GCC. AM_CFLAGS
(where -O0 is added with --disable-optimization) is passed as argument
to GCC before autoconf CFLAGS, so it is not possible to override the
default -O2. One solution is to use:
CFLAGS= ./configure --disable-optimization
i.e. remove -O2 from CFLAGS, and let autoconf add -O0.
---
acinclude.m4 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/acinclude.m4 b/acinclude.m4
index 4357c00..286340d 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -32,7 +32,7 @@ AC_DEFUN([MISC_FLAGS], [
AC_ARG_ENABLE(optimization, AC_HELP_STRING([--disable-optimization],
[disable code optimization through compiler]), [
if (test "${enableval}" = "no"); then
- misc_cflags="$misc_cflags -D_FORTIFY_SOURCE=2"
+ misc_cflags="$misc_cflags -O0"
fi
])
AC_ARG_ENABLE(debug, AC_HELP_STRING([--enable-debug],
--
1.7.9.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox