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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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."

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux