mbox series

[v2,00/14] Use RTE_LOG_LINE in drivers

Message ID 20240912082643.1532679-1-david.marchand@redhat.com (mailing list archive)
Headers
Series Use RTE_LOG_LINE in drivers |

Message

David Marchand Sept. 12, 2024, 8:26 a.m. UTC
This is a continuation of the cleanup effort in logging macros.

As a reminder of what this series is about, RTE_LOG_LINE() has been
introduced to check that the format string does not contain a trailing \n.
The goal was to prevent from:
- introducing multilines log messages (ugly and unhelpful for lambda
  users),
- introducing double \n in logs,
- introducing missing \n in logs,

I have built/maintained this series for fixing drivers/ since the
beginning of the year and I hope we can merge this series early in
24.11.

Some drivers are not converted because of multiple inconsistencies.
Those may be fixed later.

As for the changes on lib/, new macros have been introduced so that
backports trigger a build error and force the backport to adjust the
patches containing logs accordingly.


Most of the changes have been done with some scripting, but it is likely
there are some errors and I hope driver maintainers will review in
depth.


A check is added for drivers/ in the hope that new drivers don't
introduce more mess.

Yet, sometimes it is not possible to fix macros (a good example 
is base/ drivers code...), so the check skips *osdep.h headers where
calls to RTE_LOG() may be accepted.
  

Comments

David Marchand Sept. 16, 2024, 9:32 a.m. UTC | #1
On Thu, Sep 12, 2024 at 10:27 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> This is a continuation of the cleanup effort in logging macros.
>
> As a reminder of what this series is about, RTE_LOG_LINE() has been
> introduced to check that the format string does not contain a trailing \n.
> The goal was to prevent from:
> - introducing multilines log messages (ugly and unhelpful for lambda
>   users),
> - introducing double \n in logs,
> - introducing missing \n in logs,
>
> I have built/maintained this series for fixing drivers/ since the
> beginning of the year and I hope we can merge this series early in
> 24.11.
>
> Some drivers are not converted because of multiple inconsistencies.
> Those may be fixed later.
>
> As for the changes on lib/, new macros have been introduced so that
> backports trigger a build error and force the backport to adjust the
> patches containing logs accordingly.
>
>
> Most of the changes have been done with some scripting, but it is likely
> there are some errors and I hope driver maintainers will review in
> depth.
>
>
> A check is added for drivers/ in the hope that new drivers don't
> introduce more mess.
>
> Yet, sometimes it is not possible to fix macros (a good example
> is base/ drivers code...), so the check skips *osdep.h headers where
> calls to RTE_LOG() may be accepted.

One note, there is an issue with the crypto_perf_cryptodev_perf test
in (legacy) DTS.
It (non explicitly) relied on the presence of an extra empty line, and
the fact that a line would not contain special chars like : (what a
joy..).

I posted a fix for DTS.
https://patchwork.dpdk.org/project/dts/patch/20240916092515.1851927-1-david.marchand@redhat.com/
  
Patrick Robb Sept. 16, 2024, 3:13 p.m. UTC | #2
Recheck-request: iol-intel-Functional

Rerunning with David's DTS patch mentioned above.
  
David Marchand Oct. 4, 2024, 1:04 p.m. UTC | #3
On Thu, Sep 12, 2024 at 10:27 AM David Marchand
<david.marchand@redhat.com> wrote:
>
> This is a continuation of the cleanup effort in logging macros.
>
> As a reminder of what this series is about, RTE_LOG_LINE() has been
> introduced to check that the format string does not contain a trailing \n.
> The goal was to prevent from:
> - introducing multilines log messages (ugly and unhelpful for lambda
>   users),
> - introducing double \n in logs,
> - introducing missing \n in logs,
>
> I have built/maintained this series for fixing drivers/ since the
> beginning of the year and I hope we can merge this series early in
> 24.11.
>
> Some drivers are not converted because of multiple inconsistencies.
> Those may be fixed later.
>
> As for the changes on lib/, new macros have been introduced so that
> backports trigger a build error and force the backport to adjust the
> patches containing logs accordingly.
>
>
> Most of the changes have been done with some scripting, but it is likely
> there are some errors and I hope driver maintainers will review in
> depth.
>
>
> A check is added for drivers/ in the hope that new drivers don't
> introduce more mess.
>
> Yet, sometimes it is not possible to fix macros (a good example
> is base/ drivers code...), so the check skips *osdep.h headers where
> calls to RTE_LOG() may be accepted.

Series applied.