Re: [PATCH 2/3] xen: Rename asprintf() to xasprintf()

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 21/02/2022 12:23, Roger Pau Monne wrote:
> On Mon, Feb 21, 2022 at 11:28:39AM +0000, Andrew Cooper wrote:
>> On 21/02/2022 11:21, Roger Pau Monné wrote:
>>> On Mon, Feb 21, 2022 at 10:02:53AM +0000, Andrew Cooper wrote:
>>>> Coverity reports that there is a memory leak in
>>>> ioreq_server_alloc_rangesets().  This would be true if Xen's 
>>>> implementation of
>>>> asprintf() had glibc's return semantics, but it doesn't.
>>>>
>>>> Rename to xasprintf() to reduce confusion for Coverity and other 
>>>> developers.
>>> It would seem more natural to me to rename to asprintk.
>> Why?  This infrastructure doesn't emit the string to any console.
> Right, but the f in printf is for print formatted, not for where the
> output is supposed to go. So printk is the outlier and should instead
> be kprintf?
>
> I can buy into using xasprintf (also because that's what Linux does
> with kasprintf), but I don't think it's so obvious given the precedent
> of having printk instead of printf.

The naming isn't ideal, but this is Xen's local version of the thing
called asprintf() in userspace.

Naming it anything other than xasprintf() is going to be even more
confusing for people than this mess already is...

>>>  I assume
>>> there's no way for Coverity to prevent overrides with builtin models?
>>>
>>> I've been searching, but there doesn't seem to be any option to
>>> prevent overrides by builtin models?
>> No, and we absolutely wouldn't want to skip the model even if we could,
>> because that would break asprintf() analysis for userspace.
> Well, we could maybe find a way to only enable the flag for hypervisor
> code build, but anyway, it's pointless to discus if there's no flag in
> the first place.
>
> Coverity could be clever enough to check if there's an implementation
> provided for those, instead of unconditionally override with a
> model.

There is no way to disable the model for asprintf().

~Andrew