On Wed, Feb 28, 2024 at 12:45:04PM +0100, David Marchand wrote: > On Tue, Feb 27, 2024 at 7:15 PM Tyler Retzlaff > <roret...@linux.microsoft.com> wrote: > > > > On Mon, Feb 26, 2024 at 12:54:36PM -0800, Stephen Hemminger wrote: > > > On Mon, 26 Feb 2024 12:19:30 -0800 > > > Tyler Retzlaff <roret...@linux.microsoft.com> wrote: > > > > > > > RTE_LOG_LINE cannot be augmented with a prefix format and arguments > > > > without the user of RTE_LOG_LINE using the args... and ## args compiler > > > > extension to conditionally remove trailing comma when the macro receives > > > > only a single argument. > > > > > > > > Provide a new/similar macro RTE_LOG_LINE_PREFIX that accepts the prefix > > > > format and arguments as separate parameters allowing them to be expanded > > > > at the correct locations inside of RTE_FMT() allowing the rest of the > > > > non-prefix format string and arguments to be collapsed to the argument > > > > pack which can be directly forwarded with __VA_ARGS__ avoiding the need > > > > for conditional comma removal. > > > > > > > > I've done my best to manually check expansions (preprocessed) and > > > > compiled > > > > printf of the logs to validate correct output. > > > > > > > > note: due to drastic change in series i have not carried any series acks > > > > forward. > > > > > > The changes look good, you might want to add release note, update coding > > > style doc, and/or update checkpatch to discourage re-introduction. > > > > re-introduction should be protected by the CI. i know checkpatch would > > be better but i couldn't think of a good way to match an arbitrarily > > named pack ... reliably where it could potentially cause noise. > > What about a simple: > > + # forbid use of variadic argument pack extension in macros > + awk -v FOLDERS="lib drivers app examples" \ > + -v > EXPRESSIONS='#[[:space:]]*define.*[^(,[:space:]]\\.\\.\\.[[:space:]]*)' > \ > + -v RET_ON_FAIL=1 \ > + -v MESSAGE='Do not use variadic argument pack in macros' \ > + -f $(dirname $(readlink -f $0))/check-forbidden-tokens.awk \ > + "$1" || res=1 > +
I have no objection, I suppose worst case scenario if it turns out to catch things we don't want it to catch we can always ignore or remove it. I will add the check and submit a new rev. > > > -- > David Marchand
