From: Rémi Bernon <rbernon@codeweavers.com>
Subject: Re: [PATCH 1/2] wineserver: Implement thread priorities on Unix with sched_setscheduler / setpriority
Message-Id: <71062d64f23ee30b5561f1fedbc972d5cc23a730.camel@codeweavers.com>
Date: Wed, 03 Jul 2019 15:13:59 +0200
In-Reply-To: <20190703180315.a4a6babc6d29476ab4498d19@baikal.ru>
References: <20190703094119.17048-1-rbernon@codeweavers.com> <20190703180315.a4a6babc6d29476ab4498d19@baikal.ru>

On Wed, 2019-07-03 at 18:03 +0800, Dmitry Timoshkov wrote:
> Rémi Bernon <rbernon@codeweavers.com> wrote:
> 
> > diff --git a/configure b/configure
> > index 3b6d744c391..e89be467e2e 100755
> > --- a/configure
> > +++ b/configure
> 
> There is no need to include a diff for auto-generated file.

Alright but the file is committed and only regenerated with
autoreconf -fi, right? I believe I had to do it manually.

> > +    if (thread->process->priority == PROCESS_PRIOCLASS_REALTIME)
> > +    {
> > +#ifdef HAVE_SCHED_SETSCHEDULER
> > +        struct sched_param param;
> > +        if (sched_getparam( thread->unix_tid, &param ) != 0)
> > +            goto error;
> 
> This 'goto' is useless.

The error handling code path currently returns 0 in order not to
introduce any behavior change, but it should perhaps return an error
code instead. This goto was there to indicate the error codepath is
used in opposition to the other return 0 statements which are the
"success" codepath. Maybe a return 0 with a comment would make it more
clear, as for the last return 0 as well.
-- 
Rémi Bernon <rbernon@codeweavers.com>