All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: Ivan Lazaric <ivan.lazaric1@gmail.com>, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v2] perf stat: reject --field-separator and --json-output combination
Date: Wed, 8 Jul 2026 16:43:13 -0700	[thread overview]
Message-ID: <ak7gkZaEahlRlgvA@google.com> (raw)
In-Reply-To: <20260708133656.107611F000E9@smtp.kernel.org>

On Wed, Jul 08, 2026 at 01:36:55PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Missing explicit inclusion of `<stdio.h>` for standard I/O functions and types.
> --
> 
> --- Patch [1]: [PATCH v2] perf stat: reject --field-separator and --json-output combination ---
> commit 14ca4ce3700a93ed48a3336a633292f7e45ae462
> Author: Ivan Lazaric <ivan.lazaric1@gmail.com>
> 
> perf stat: reject --field-separator and --json-output combination
> 
> This patch explicitly rejects using both the --field-separator and
> --json-output options in perf stat. It adds a check to output an error
> message and exit cleanly, preventing malformed mixed formats.
> 
> > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > index a04466ea3b0a0..6ec82752d71c6 100644
> > --- a/tools/perf/builtin-stat.c
> > +++ b/tools/perf/builtin-stat.c
> > @@ -2643,6 +2643,13 @@ int cmd_stat(int argc, const char **argv)
> >  
> >  	stat_config.aggr_mode = opt_aggr_mode_to_aggr_mode(&opt_mode);
> >  
> > +	if (stat_config.csv_sep && stat_config.json_output) {
> > +		fprintf(stderr, "cannot use both --field-separator and --json-output\n");
>                 ^^^^^^^
> 
> [Severity: Low]
> Could this cause a build failure when compiling perf with musl libc?
> 
> While glibc often implicitly includes standard headers, musl has stricter
> header separation.
> 
> Since this code introduces a new call to fprintf and uses stderr, does
> builtin-stat.c need an explicit #include <stdio.h> in its global includes
> block to guarantee musl compatibility?

It should be fine as it's used in other places already.

Thanks,
Namhyung

> 
> > +		parse_options_usage(stat_usage, stat_options, "x", 1);
> > +		parse_options_usage(NULL, stat_options, "j", 1);
> > +		goto out;
> > +	}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260708131359.71732-3-ivan.lazaric1@gmail.com?part=1

  reply	other threads:[~2026-07-08 23:43 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-05  0:58 [PATCH] perf stat: reject --field-separator and --json-output combination Ivan Lazaric
2026-07-08  6:21 ` Namhyung Kim
2026-07-08 13:14   ` [PATCH v2] " Ivan Lazaric
2026-07-08 13:36     ` sashiko-bot
2026-07-08 23:43       ` Namhyung Kim [this message]
2026-07-10  0:41     ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ak7gkZaEahlRlgvA@google.com \
    --to=namhyung@kernel.org \
    --cc=ivan.lazaric1@gmail.com \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.