[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201029110605.GI16862@leoy-ThinkPad-X240s>
Date: Thu, 29 Oct 2020 19:06:05 +0800
From: Leo Yan <leo.yan@...aro.org>
To: André Przywara <andre.przywara@....com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
Jiri Olsa <jolsa@...hat.com>,
Namhyung Kim <namhyung@...nel.org>,
Dave Martin <Dave.Martin@....com>,
Wei Li <liwei391@...wei.com>,
James Clark <james.clark@....com>, Al Grant <Al.Grant@....com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 06/21] perf arm-spe: Refactor printing string to buffer
On Thu, Oct 29, 2020 at 10:54:37AM +0000, André Przywara wrote:
> On 29/10/2020 10:51, Leo Yan wrote:
> > Hi Andre,
> >
> > On Thu, Oct 29, 2020 at 10:23:39AM +0000, Andr� Przywara wrote:
> >
> > [...]
> >
> >>> +static int arm_spe_pkt_snprintf(int *err, char **buf_p, size_t *blen,
> >>> + const char *fmt, ...)
> >>> +{
> >>> + va_list ap;
> >>> + int ret;
> >>> +
> >>> + va_start(ap, fmt);
> >>> + ret = vsnprintf(*buf_p, *blen, fmt, ap);
> >>> + va_end(ap);
> >>> +
> >>> + if (ret < 0) {
> >>> + if (err && !*err)
> >>> + *err = ret;
> >>> + } else {
> >>> + *buf_p += ret;
> >>> + *blen -= ret;
> >>> + }
> >>> +
> >>> + return ret;
> >>> +}
> >>
> >> So this now implements the old behaviour of ignoring previous errors, in
> >> all cases, since we don't check for errors and bail out in the callers.
> >>
> >> If you simply check for validity of err and for it being 0 before
> >> proceeding with the va_start() above, this should be fixed.
> >
> > I think you are suggesting below code, could you take a look for it
> > before I proceed to respin new patch?>
> > static int arm_spe_pkt_snprintf(int *err, char **buf_p, size_t *blen,
> > const char *fmt, ...)
> > {
> > va_list ap;
> > int ret;
> >
> > /* Bail out if any error occurred */
> > if (err && *err)
> > return *err;
> >
> > va_start(ap, fmt);
> > ret = vsnprintf(*buf_p, *blen, fmt, ap);
> > va_end(ap);
> >
> > if (ret < 0) {
> > if (err && !*err)
> > *err = ret;
> > } else {
> > *buf_p += ret;
> > *blen -= ret;
> > }
> >
> > return ret;
> > }
>
> Yes, this is what I had in mind.
Thanks for confirmation, Andre.
Leo
Powered by blists - more mailing lists