Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200707003937.187125-1-alainm@chromium.org> <4B0E4627-26D8-4D70-ABE9-6CB7A9E86D35@holtmann.org>
In-Reply-To: <4B0E4627-26D8-4D70-ABE9-6CB7A9E86D35@holtmann.org>
From:   Alain Michaud <alainmichaud@google.com>
Date:   Tue, 7 Jul 2020 10:07:23 -0400
Message-ID: <CALWDO_Vq7t-6sX9UWravhcb64K_2LpsD-xGZsZec__2VcYwBgw@mail.gmail.com>
Subject: Re: [PATCH v1] Bluetooth: create CONFIG_BT_DEBUG_FUNC_NAME
To:     Marcel Holtmann <marcel@holtmann.org>
Cc:     Alain Michaud <alainm@chromium.org>,
        linux-bluetooth <linux-bluetooth@vger.kernel.org>,
        Abhishek Pandit-Subedi <abhishekpandit@chromium.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-bluetooth-owner@vger.kernel.org
Precedence: bulk

Hi Marcel

On Tue, Jul 7, 2020 at 2:32 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Alain,
>
> > Creates a CONFIG_BT_DEBUG_FUNC_NAME option to include function names in
> > debug statements.
> >
> > Unlike other platforms __func__ isn't a string literal so it cannot be
> > automatically concatenated by the pre-processor.  As a result, the
> > function name is passed as a parameter to the tracing function.  Since
> > pr_debug is a function like macro, the normal expansion of BT_PREFIX_PA=
RAM
> > does not work as it gets processed within the first parameter as well,
> > for this reason, BT_DBG is split into two versions.
> >
> > This patch was built tested with all 4 possible combinations of
> > CONFIG_BT_DEBUG_FUNC_NAME and CONFIG_BT_FEATURE_DEBUG configurations.
>
> can we please limit this to FEATURE_DEBUG since dynamic debug doesn=E2=80=
=99t need it. It can switch on function name debugging on a per debug state=
ment. And even for FEATURE_DEBUG I would rather have it optional that can b=
e enabled when needed via the experimental feature itself.
I agree on making this dependent on FEATURE_DEBUG as it may simplify
the configuration, but I don't think I like having this enabled via an
experimental feature as it complicates the tracing macros quite a bit
for no good reason.  I don't see a scenario where someone would turn
on CONFIG_DEBUG_FUNC_NAME but not want it enabled.

>
> Regards
>
> Marcel
>