Re: [PATCH] virprocess: Provide non-Linux stubs for virProcessGet{Stat, Sched}Info
On Fri, 7 Jan 2022, Daniel P. Berrangé wrote:
On Fri, Jan 07, 2022 at 05:24:18PM +0530, Ani Sinha wrote:
>
>
> On Fri, 7 Jan 2022, Ján Tomko wrote:
>
> > On a Friday in 2022, Ani Sinha wrote:
> > > On Fri, 7 Jan 2022, Michal Prívozník wrote:
> > > > I don't think so. Just like we've discussed under one patch
of yours, a
> > > > function should either report error in all cases or none. And in case
of
> > > > virProcessGetSchedInfo() the linux version does report error
> > >
> > > I see your point but there is also a bug in that function - not all error
> > > paths report errors. For example, !proc and !lines cases. We need to fix
> > > that.
> > >
> >
> > I don't see a !proc error path in virProcessGetSchedInfo.
> >
>
> if (tid)
> proc = g_strdup_printf("/proc/%d/task/%d/sched", (int) pid, (int)
> tid);
> else
> proc = g_strdup_printf("/proc/%d/sched", (int) pid);
> if (!proc)
> return -1; <=== not reported
g_strdup_printf can't fail unless we mangled the printf format string
(which we havent), so that 'if (!proc)' check is redundant / unreachable
I am ok with Michal's patch that removes this check. However, such
assumptutions does makes me nervous. Since we do not really have
control over the library that implements g_strdup_printf, what if the
"can't fail" logic changes one day? This can happen deliberately or due to
some bug in the library. Does it not make sense to be defensive as the
consumer of this function and write code that is future proof?