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 :|