Hi Andrei
On Tue, May 8, 2012 at 3:49 PM, Andrei Emeltchenko
<andrei.emeltchenko.news@xxxxxxxxx> wrote:
> Hi David,
>
> On Tue, May 08, 2012 at 03:28:25PM +0200, David Herrmann wrote:
>> batostr() is not thread-safe so we _must_ use ba2str() in all un-protected
>> paths (which is pretty everywhere in our source).
>>
>> All places where BT_DBG() is used we call ba2str() as parameter so the
>> static buffer is unused if debug is disabled during compilation and gcc
>> should be clever enough to remove the buffers.
>>
>> Reported-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
>> Signed-off-by: David Herrmann <dh.herrmann@xxxxxxxxxxxxxx>
>> ---
>> net/bluetooth/bnep/core.c | 4 +++-
>> net/bluetooth/cmtp/core.c | 3 ++-
>> net/bluetooth/hci_conn.c | 9 ++++++---
>> net/bluetooth/hci_core.c | 32 ++++++++++++++++++++++----------
>> net/bluetooth/hci_event.c | 17 +++++++++++------
>> net/bluetooth/hci_sysfs.c | 14 ++++++++++----
>> net/bluetooth/hidp/core.c | 4 ++--
>> net/bluetooth/l2cap_core.c | 23 ++++++++++++++---------
>> net/bluetooth/rfcomm/core.c | 15 +++++++++------
>> net/bluetooth/rfcomm/sock.c | 10 ++++++----
>> net/bluetooth/rfcomm/tty.c | 10 +++++++---
>> net/bluetooth/sco.c | 20 ++++++++++++++------
>> 12 files changed, 106 insertions(+), 55 deletions(-)
>>
>> diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
>> index a779ec7..03a3cb3 100644
>> --- a/net/bluetooth/bnep/core.c
>> +++ b/net/bluetooth/bnep/core.c
>> @@ -166,6 +166,7 @@ static int bnep_ctrl_set_netfilter(struct bnep_session *s, __be16 *data, int len
>> static int bnep_ctrl_set_mcfilter(struct bnep_session *s, u8 *data, int len)
>> {
>> int n;
>> + char babuf1[BDADDR_STRLEN], babuf2[BDADDR_STRLEN];
>
> NAK in this form. You are always allocating unneeded memory.
That's on the stack. I think gcc should be smart enough to _not_
allocate this on code paths not using it. Furthermore, there is only
one other way to do this and this is introducing a new format
specifier for snprintf() that takes as argument a bdaddr_t but I tried
to avoid this.
I also don't know what the problem is with using the stack for both
buffers? That's definitely no speed problem so you must be concerned
about stack size. But again, both are not even available when
disabling debug and thats just 18 bytes per buffer...
Please be more specific about what's bothering you here.
> PS: It is not OK to put extra #ifdef's as well.
>
> Best regards
> Andrei Emeltchenko
Cheers
David
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
