Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220323000654.3157833-1-luiz.dentz@gmail.com>
 <20220323000654.3157833-4-luiz.dentz@gmail.com> <DE87AE95-85DD-4732-8D49-F6E212F836B6@holtmann.org>
In-Reply-To: <DE87AE95-85DD-4732-8D49-F6E212F836B6@holtmann.org>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Wed, 23 Mar 2022 12:58:24 -0700
Message-ID: <CABBYNZK+Sh_r5SW7ot=QR2gweKf6MtGDsKjGdRGWOmKEP-ny5g@mail.gmail.com>
Subject: Re: [PATCH BlueZ v2 4/9] adapter: Don't use DBG in mgmt_debug
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Marcel,

On Wed, Mar 23, 2022 at 3:37 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Luiz,
>
> > mgmt_debug callback is used to print debug strings from mgmt instances
> > which includes the file and function names so using DBG would add yet
> > another set of file and function prefixes which makes the logs
> > confusing.
> > ---
> > src/adapter.c | 8 +++-----
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/adapter.c b/src/adapter.c
> > index 97ce26f8e..6680c5410 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -10327,9 +10327,8 @@ static void read_version_complete(uint8_t status, uint16_t length,
> >
> > static void mgmt_debug(const char *str, void *user_data)
> > {
> > -     const char *prefix = user_data;
> > -
> > -     info("%s%s", prefix, str);
> > +     if (DBG_IS_ENABLED())
> > +             btd_debug(0xffff, "%s", str);
> > }
> >
> > int adapter_init(void)
> > @@ -10342,8 +10341,7 @@ int adapter_init(void)
> >               return -EIO;
> >       }
> >
> > -     if (getenv("MGMT_DEBUG"))
> > -             mgmt_set_debug(mgmt_primary, mgmt_debug, "mgmt: ", NULL);
> > +     mgmt_set_debug(mgmt_primary, mgmt_debug, NULL, NULL);
>
> oh no. This is crazy. Please re-think this and what computational overhead you are introducing.

I considered moving DBG_IS_ENABLED() in place of getenv("MGMT_DEBUG")
so that would be use just once per adapter, the problem is that
wouldn't work with:

case SIGUSR2:
__btd_toggle_debug();

Or perhaps I just need yet another match that adds the file and
function name so we can strip them out of DBG_IDX.

> Regards
>
> Marcel
>


-- 
Luiz Augusto von Dentz