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?