Hi David,
On Tue, May 08, 2012 at 04:02:39PM +0200, David Herrmann wrote:
> 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.
Those extra variables does not look necessary. What about the method
suggested by Johannes using %pM
Best regards
Andrei Emeltchenko
--
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
