Re: [PATCH] virprocess: Provide non-Linux stubs for virProcessGet{Stat, Sched}Info

On Fri, Jan 07, 2022 at 06:04:04PM +0530, Ani Sinha wrote: > > > 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? Their API spec guarantees we are safe https://docs.gtk.org/glib/func.strdup_printf.html "The returned string is guaranteed to be non-NULL, unless format contains %lc or %ls conversions, which can fail if no multibyte representation is available for the given character."