Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <4941e4a6d898a4545ff319b6b74442e18ca8025a.camel@gmail.com>
Subject: Re: [RFC PATCH v2 01/10] perf workqueue: threadpool creation and
 destruction
From:   Riccardo Mancini <rickyman7@gmail.com>
To:     Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>,
        Namhyung Kim <namhyung@kernel.org>
Cc:     Ian Rogers <irogers@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Ingo Molnar <mingo@redhat.com>,
        Mark Rutland <mark.rutland@arm.com>,
        Jiri Olsa <jolsa@redhat.com>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        linux-perf-users <linux-perf-users@vger.kernel.org>,
        Alexey Bayduraev <alexey.v.bayduraev@linux.intel.com>
Date:   Wed, 11 Aug 2021 19:55:27 +0200
In-Reply-To: <YRLgYDKRUfwLSyHF@kernel.org>
References: <cover.1627657061.git.rickyman7@gmail.com>
         <4f0cd6c8e77c0b4f4d4b8d553a7032757b976e61.1627657061.git.rickyman7@gmail.com>
         <CAM9d7ci0LcdTZyDu5sxGkaS07Z-m4hvr2xU4TDK_aTRAJm7b7A@mail.gmail.com>
         <25278c08762593a5b0bd1873f6c0745c7ad97016.camel@gmail.com>
         <CAM9d7chuFG+cYPx34abvmciN0afUDqJ3uWo9Bsh-9jjLtQmWqQ@mail.gmail.com>
         <YRLgYDKRUfwLSyHF@kernel.org>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.40.3 (3.40.3-1.fc34) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

On Tue, 2021-08-10 at 17:24 -0300, Arnaldo Carvalho de Melo wrote:
> Em Tue, Aug 10, 2021 at 11:54:19AM -0700, Namhyung Kim escreveu:
> > On Mon, Aug 9, 2021 at 3:30 AM Riccardo Mancini <rickyman7@gmail.com> wrote:
> > > On Fri, 2021-08-06 at 19:24 -0700, Namhyung Kim wrote:
> > > > > +/**
> > > > > + * threadpool__strerror - print message regarding given @err in @pool
> > > > > + *
> > > > > + * Buffer size should be at least THREADPOOL_STRERR_BUFSIZE bytes.
> > > > > + */
> > > > > +int threadpool__strerror(struct threadpool *pool __maybe_unused, int
> > > > > err,
> > > > > char *buf, size_t size)
> > > > > +{
> > > > > +       char sbuf[STRERR_BUFSIZE], *emsg;
> > > > > +
> > > > > +       emsg = str_error_r(err, sbuf, sizeof(sbuf));
> > > > > +       return scnprintf(buf, size, "Error: %s.\n", emsg);
> > > > > +}
> > > > > +
> > > > > +/**
> > > > > + * threadpool__new_strerror - print message regarding @err_ptr
> > > > > + *
> > > > > + * Buffer size should be at least THREADPOOL_STRERR_BUFSIZE bytes.
> > > > > + */
> > > > > +int threadpool__new_strerror(struct threadpool *err_ptr, char *buf,
> > > > > size_t
> > > > > size)
> > > > > +{
> > > > > +       return threadpool__strerror(err_ptr, PTR_ERR(err_ptr), buf,
> > > > > size);
> > > > > +}
> 
> > > > Why two different functions?
> 
> > > Since when new fails you don't have a err number, just an err_ptr so it's
> > > not
> > > very clear how to call threadpool__strerror. Therefore I made a wrapper to
> > > remove any ambiguity.
> > 
> > What do you mean by "when new fails"?
> 
> I think 'new' is 'constructor', i.e. something__new() returns a newly
> created object and this not an error number, so he uses ERR_PTR() and
> then he needs to pass it to the 'strerror' specific to the
> threadpool__new, which will use PTR_ERR() to get an integer, and then
> map that to a proper error string, right?

Correct. 
threadpool__new_strerror is not really needed since one could use
threadpool__strerror directly, but then I would need to handle all possible ways
it could be called (e.g. (NULL, PTR_ERR(err_ptr)), (err_ptr, 0), (err_ptr,
PTR_ERR(err_ptr)), so I thought it was better to just provide a strerror that
only took the err_ptr.
Maybe I can provide it as an inline in the header, or as a macro.

Riccardo

> 
> - Arnaldo