[PATCH 0/3] Misc coverity fixes and tweaks

[PATCH 0/3] Misc coverity fixes and tweaks

[Andrew Cooper]

Andrew Cooper (3):
  tests/resource: Initialise gnttab before
    xenforeignmemory_map_resource()
  xen: Rename asprintf() to xasprintf()
  CI: Coverity tweaks

 .github/workflows/coverity.yml       | 14 ++++++++------
 tools/tests/resource/test-resource.c |  2 +-
 xen/common/ioreq.c                   | 16 ++++++++++------
 xen/common/vsprintf.c                | 11 ++++++-----
 xen/include/xen/lib.h                |  4 ++--
 5 files changed, 27 insertions(+), 20 deletions(-)

-- 
2.11.0

[PATCH 1/3] tests/resource: Initialise gnttab before xenforeignmemory_map_resource()

[Andrew Cooper]

It the 'addr' input to mmap(), and currently consuming stack rubble.

Coverity-ID: 1500115
Fixes: c7a7f14b9299 ("tests/resource: Extend to check that the grant frames are mapped correctly")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 tools/tests/resource/test-resource.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/tests/resource/test-resource.c b/tools/tests/resource/test-resource.c
index 0557f8a1b585..189353ebcb43 100644
--- a/tools/tests/resource/test-resource.c
+++ b/tools/tests/resource/test-resource.c
@@ -24,7 +24,7 @@ static void test_gnttab(uint32_t domid, unsigned int nr_frames,
                         unsigned long gfn)
 {
     xenforeignmemory_resource_handle *res;
-    grant_entry_v1_t *gnttab;
+    grant_entry_v1_t *gnttab = NULL;
     size_t size;
     int rc;
     uint32_t refs[nr_frames], domids[nr_frames];
-- 
2.11.0

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

[Andrew Cooper]

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.

While at it, fix style issues.  Rearrange ioreq_server_alloc_rangesets() to
use a tabulated switch statement, and not to have a trailing space in the
rangeset name for an unknown range type.

Coverity-ID: 1472735
Coverity-ID: 1500265
Fixes: 780e918a2e54 ("add an implentation of asprintf() for xen")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: George Dunlap <George.Dunlap@eu.citrix.com>
CC: Ian Jackson <iwj@xenproject.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Wei Liu <wl@xen.org>
CC: Julien Grall <julien@xen.org>
CC: Paul Durrant <paul@xen.org>
---
 xen/common/ioreq.c    | 16 ++++++++++------
 xen/common/vsprintf.c | 11 ++++++-----
 xen/include/xen/lib.h |  4 ++--
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/xen/common/ioreq.c b/xen/common/ioreq.c
index 689d256544c8..5c94e74293ce 100644
--- a/xen/common/ioreq.c
+++ b/xen/common/ioreq.c
@@ -501,13 +501,17 @@ static int ioreq_server_alloc_rangesets(struct ioreq_server *s,
 
     for ( i = 0; i < NR_IO_RANGE_TYPES; i++ )
     {
-        char *name;
+        char *name, *type;
 
-        rc = asprintf(&name, "ioreq_server %d %s", id,
-                      (i == XEN_DMOP_IO_RANGE_PORT) ? "port" :
-                      (i == XEN_DMOP_IO_RANGE_MEMORY) ? "memory" :
-                      (i == XEN_DMOP_IO_RANGE_PCI) ? "pci" :
-                      "");
+        switch ( i )
+        {
+        case XEN_DMOP_IO_RANGE_PORT:   type = " port";   break;
+        case XEN_DMOP_IO_RANGE_MEMORY: type = " memory"; break;
+        case XEN_DMOP_IO_RANGE_PCI:    type = " pci";    break;
+        default:                       type = "";        break;
+        }
+
+        rc = xasprintf(&name, "ioreq_server %d%s", id, type);
         if ( rc )
             goto fail;
 
diff --git a/xen/common/vsprintf.c b/xen/common/vsprintf.c
index 185a4bd5610a..b278961cc387 100644
--- a/xen/common/vsprintf.c
+++ b/xen/common/vsprintf.c
@@ -859,7 +859,7 @@ int scnprintf(char * buf, size_t size, const char *fmt, ...)
 EXPORT_SYMBOL(scnprintf);
 
 /**
- * vasprintf - Format a string and allocate a buffer to place it in
+ * xvasprintf - Format a string and allocate a buffer to place it in
  *
  * @bufp: Pointer to a pointer to receive the allocated buffer
  * @fmt: The format string to use
@@ -870,7 +870,7 @@ EXPORT_SYMBOL(scnprintf);
  * guaranteed to be null terminated. The memory is allocated
  * from xenheap, so the buffer should be freed with xfree().
  */
-int vasprintf(char **bufp, const char *fmt, va_list args)
+int xvasprintf(char **bufp, const char *fmt, va_list args)
 {
     va_list args_copy;
     size_t size;
@@ -891,7 +891,7 @@ int vasprintf(char **bufp, const char *fmt, va_list args)
 }
 
 /**
- * asprintf - Format a string and place it in a buffer
+ * xasprintf - Format a string and place it in a buffer
  * @bufp: Pointer to a pointer to receive the allocated buffer
  * @fmt: The format string to use
  * @...: Arguments for the format string
@@ -901,14 +901,15 @@ int vasprintf(char **bufp, const char *fmt, va_list args)
  * guaranteed to be null terminated. The memory is allocated
  * from xenheap, so the buffer should be freed with xfree().
  */
-int asprintf(char **bufp, const char *fmt, ...)
+int xasprintf(char **bufp, const char *fmt, ...)
 {
     va_list args;
     int i;
 
     va_start(args, fmt);
-    i=vasprintf(bufp,fmt,args);
+    i = xvasprintf(bufp, fmt, args);
     va_end(args);
+
     return i;
 }
 
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index c6987973bf88..aea60d292724 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -158,9 +158,9 @@ extern int scnprintf(char * buf, size_t size, const char * fmt, ...)
     __attribute__ ((format (printf, 3, 4)));
 extern int vscnprintf(char *buf, size_t size, const char *fmt, va_list args)
     __attribute__ ((format (printf, 3, 0)));
-extern int asprintf(char ** bufp, const char * fmt, ...)
+extern int xasprintf(char **bufp, const char *fmt, ...)
     __attribute__ ((format (printf, 2, 3)));
-extern int vasprintf(char ** bufp, const char * fmt, va_list args)
+extern int xvasprintf(char **bufp, const char *fmt, va_list args)
     __attribute__ ((format (printf, 2, 0)));
 
 long simple_strtol(
-- 
2.11.0

[PATCH 3/3] CI: Coverity tweaks

[Andrew Cooper]

 * Use workflow_dispatch to allow manual creation of the job.
 * Use parallel builds.  The workers have two vCPUs.
 * Shrink the dependency list further.  build-essential covers make and gcc,
   while bridge-utils and iproute2 are runtime dependencies not build
   dependencies.  Alter bzip2 to libbz2-dev.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 .github/workflows/coverity.yml | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 9d04b56fd31d..6e7b81e74f72 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -2,6 +2,7 @@ name: Coverity Scan
 
 # We only want to test official release code, not every pull request.
 on:
+  workflow_dispatch:
   schedule:
     - cron: '18 9 * * WED,SUN' # Bi-weekly at 9:18 UTC
 
@@ -11,11 +12,11 @@ jobs:
     steps:
     - name: Install build dependencies
       run: |
-        sudo apt-get install -y wget git gawk bridge-utils \
-          iproute2 bzip2 build-essential \
-          make gcc zlib1g-dev libncurses5-dev iasl \
-          libbz2-dev e2fslibs-dev git-core uuid-dev ocaml \
-          ocaml-findlib xz-utils libyajl-dev \
+        sudo apt-get install -y wget git gawk \
+          libbz2-dev build-essential \
+          zlib1g-dev libncurses5-dev iasl \
+          libbz2-dev e2fslibs-dev uuid-dev ocaml \
+          ocaml-findlib libyajl-dev \
           autoconf libtool liblzma-dev \
           python3-dev golang python-dev libsystemd-dev
 
@@ -31,7 +32,7 @@ jobs:
 
     - name: Pre build stuff
       run: |
-        make mini-os-dir
+        make -j`nproc` mini-os-dir
 
     - uses: vapier/coverity-scan-action@v1
       with:
@@ -39,3 +40,4 @@ jobs:
         project: XenProject
         email: ${{ secrets.COVERITY_SCAN_EMAIL }}
         token: ${{ secrets.COVERITY_SCAN_TOKEN }}
+        command: make -j`nproc` build
-- 
2.11.0

Re: [PATCH 3/3] CI: Coverity tweaks

[Roger Pau Monné]

On Mon, Feb 21, 2022 at 10:02:54AM +0000, Andrew Cooper wrote:
>  * Use workflow_dispatch to allow manual creation of the job.
>  * Use parallel builds.  The workers have two vCPUs.
>  * Shrink the dependency list further.  build-essential covers make and gcc,
>    while bridge-utils and iproute2 are runtime dependencies not build
>    dependencies.  Alter bzip2 to libbz2-dev.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  .github/workflows/coverity.yml | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
> index 9d04b56fd31d..6e7b81e74f72 100644
> --- a/.github/workflows/coverity.yml
> +++ b/.github/workflows/coverity.yml
> @@ -2,6 +2,7 @@ name: Coverity Scan
>  
>  # We only want to test official release code, not every pull request.
>  on:
> +  workflow_dispatch:
>    schedule:
>      - cron: '18 9 * * WED,SUN' # Bi-weekly at 9:18 UTC
>  
> @@ -11,11 +12,11 @@ jobs:
>      steps:
>      - name: Install build dependencies
>        run: |
> -        sudo apt-get install -y wget git gawk bridge-utils \
> -          iproute2 bzip2 build-essential \
> -          make gcc zlib1g-dev libncurses5-dev iasl \
> -          libbz2-dev e2fslibs-dev git-core uuid-dev ocaml \
> -          ocaml-findlib xz-utils libyajl-dev \
> +        sudo apt-get install -y wget git gawk \
> +          libbz2-dev build-essential \
> +          zlib1g-dev libncurses5-dev iasl \
> +          libbz2-dev e2fslibs-dev uuid-dev ocaml \
> +          ocaml-findlib libyajl-dev \
>            autoconf libtool liblzma-dev \
>            python3-dev golang python-dev libsystemd-dev
>  
> @@ -31,7 +32,7 @@ jobs:
>  
>      - name: Pre build stuff
>        run: |
> -        make mini-os-dir
> +        make -j`nproc` mini-os-dir
>  
>      - uses: vapier/coverity-scan-action@v1
>        with:
> @@ -39,3 +40,4 @@ jobs:
>          project: XenProject
>          email: ${{ secrets.COVERITY_SCAN_EMAIL }}
>          token: ${{ secrets.COVERITY_SCAN_TOKEN }}
> +        command: make -j`nproc` build

There's already a 'command:' parameter set just before 'project:'. Are
we OK with using plain build?

If so we would have to disable docs build and stubdom? We don't want
to analyze all the newlib &c that's build as part of stubdoms?

Anyway, the switch from `make xen tools && make -C extras/mini-os/`
to `make build` needs to be explained in the commit message IMO.

Thanks, Roger.

Re: [PATCH 1/3] tests/resource: Initialise gnttab before xenforeignmemory_map_resource()

[Roger Pau Monné]

On Mon, Feb 21, 2022 at 10:02:52AM +0000, Andrew Cooper wrote:
> It the 'addr' input to mmap(), and currently consuming stack rubble.
> 
> Coverity-ID: 1500115
> Fixes: c7a7f14b9299 ("tests/resource: Extend to check that the grant frames are mapped correctly")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

Re: [PATCH 3/3] CI: Coverity tweaks

[Andrew Cooper]

On 21/02/2022 10:37, Roger Pau Monné wrote:
> On Mon, Feb 21, 2022 at 10:02:54AM +0000, Andrew Cooper wrote:
>>  * Use workflow_dispatch to allow manual creation of the job.
>>  * Use parallel builds.  The workers have two vCPUs.
>>  * Shrink the dependency list further.  build-essential covers make and gcc,
>>    while bridge-utils and iproute2 are runtime dependencies not build
>>    dependencies.  Alter bzip2 to libbz2-dev.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>>  .github/workflows/coverity.yml | 14 ++++++++------
>>  1 file changed, 8 insertions(+), 6 deletions(-)
>>
>> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
>> index 9d04b56fd31d..6e7b81e74f72 100644
>> --- a/.github/workflows/coverity.yml
>> +++ b/.github/workflows/coverity.yml
>> @@ -2,6 +2,7 @@ name: Coverity Scan
>>  
>>  # We only want to test official release code, not every pull request.
>>  on:
>> +  workflow_dispatch:
>>    schedule:
>>      - cron: '18 9 * * WED,SUN' # Bi-weekly at 9:18 UTC
>>  
>> @@ -11,11 +12,11 @@ jobs:
>>      steps:
>>      - name: Install build dependencies
>>        run: |
>> -        sudo apt-get install -y wget git gawk bridge-utils \
>> -          iproute2 bzip2 build-essential \
>> -          make gcc zlib1g-dev libncurses5-dev iasl \
>> -          libbz2-dev e2fslibs-dev git-core uuid-dev ocaml \
>> -          ocaml-findlib xz-utils libyajl-dev \
>> +        sudo apt-get install -y wget git gawk \
>> +          libbz2-dev build-essential \
>> +          zlib1g-dev libncurses5-dev iasl \
>> +          libbz2-dev e2fslibs-dev uuid-dev ocaml \
>> +          ocaml-findlib libyajl-dev \
>>            autoconf libtool liblzma-dev \
>>            python3-dev golang python-dev libsystemd-dev
>>  
>> @@ -31,7 +32,7 @@ jobs:
>>  
>>      - name: Pre build stuff
>>        run: |
>> -        make mini-os-dir
>> +        make -j`nproc` mini-os-dir
>>  
>>      - uses: vapier/coverity-scan-action@v1
>>        with:
>> @@ -39,3 +40,4 @@ jobs:
>>          project: XenProject
>>          email: ${{ secrets.COVERITY_SCAN_EMAIL }}
>>          token: ${{ secrets.COVERITY_SCAN_TOKEN }}
>> +        command: make -j`nproc` build
> There's already a 'command:' parameter set just before 'project:'.

Oh, so there is.

> Are
> we OK with using plain build?
>
> If so we would have to disable docs build and stubdom? We don't want
> to analyze all the newlib &c that's build as part of stubdoms?

The problem I was trying to work around there was that xen&tools turn
into *-install so we also spend time shuffling binaries around the build
environment.

What we actually want is:

make -j`nproc` build-xen build-tools && make -j`nproc` -C extras/mini-os/

~Andrew

[PATCH v2] CI: Coverity tweaks

[Andrew Cooper]

 * Use workflow_dispatch to allow manual creation of the job.
 * Use parallel builds; the workers have two vCPUs.  Also, use the build-*
   targets rather than the ones which expand to dist-*.
 * Shrink the dependency list further.  build-essential covers make and gcc,
   while bridge-utils and iproute2 are runtime dependencies not build
   dependencies.  Alter bzip2 to libbz2-dev.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Roger Pau Monné <roger.pau@citrix.com>

v2:
 * Merge with existing command:
---
 .github/workflows/coverity.yml | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
index 9d04b56fd31d..427fb86f947f 100644
--- a/.github/workflows/coverity.yml
+++ b/.github/workflows/coverity.yml
@@ -2,6 +2,7 @@ name: Coverity Scan
 
 # We only want to test official release code, not every pull request.
 on:
+  workflow_dispatch:
   schedule:
     - cron: '18 9 * * WED,SUN' # Bi-weekly at 9:18 UTC
 
@@ -11,11 +12,11 @@ jobs:
     steps:
     - name: Install build dependencies
       run: |
-        sudo apt-get install -y wget git gawk bridge-utils \
-          iproute2 bzip2 build-essential \
-          make gcc zlib1g-dev libncurses5-dev iasl \
-          libbz2-dev e2fslibs-dev git-core uuid-dev ocaml \
-          ocaml-findlib xz-utils libyajl-dev \
+        sudo apt-get install -y wget git gawk \
+          libbz2-dev build-essential \
+          zlib1g-dev libncurses5-dev iasl \
+          libbz2-dev e2fslibs-dev uuid-dev ocaml \
+          ocaml-findlib libyajl-dev \
           autoconf libtool liblzma-dev \
           python3-dev golang python-dev libsystemd-dev
 
@@ -31,11 +32,11 @@ jobs:
 
     - name: Pre build stuff
       run: |
-        make mini-os-dir
+        make -j`nproc` mini-os-dir
 
     - uses: vapier/coverity-scan-action@v1
       with:
-        command: make xen tools && make -C extras/mini-os/
+        command: make -j`nproc` build-xen build-tools && make -j`nproc` -C extras/mini-os/
         project: XenProject
         email: ${{ secrets.COVERITY_SCAN_EMAIL }}
         token: ${{ secrets.COVERITY_SCAN_TOKEN }}
-- 
2.11.0

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

[Roger Pau Monné]

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

Thanks, Roger.

Re: [PATCH v2] CI: Coverity tweaks

[Roger Pau Monné]

On Mon, Feb 21, 2022 at 11:14:54AM +0000, Andrew Cooper wrote:
>  * Use workflow_dispatch to allow manual creation of the job.

I guess such manual creation requires some kind of superpower
credentials on the github repo?

>  * Use parallel builds; the workers have two vCPUs.  Also, use the build-*
>    targets rather than the ones which expand to dist-*.
>  * Shrink the dependency list further.  build-essential covers make and gcc,
>    while bridge-utils and iproute2 are runtime dependencies not build
>    dependencies.  Alter bzip2 to libbz2-dev.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

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

[Andrew Cooper]

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.

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

~Andrew

Re: [PATCH v2] CI: Coverity tweaks

[Andrew Cooper]

On 21/02/2022 11:22, Roger Pau Monné wrote:
> On Mon, Feb 21, 2022 at 11:14:54AM +0000, Andrew Cooper wrote:
>>  * Use workflow_dispatch to allow manual creation of the job.
> I guess such manual creation requires some kind of superpower
> credentials on the github repo?

I'd expect its open to project members.

>>  * Use parallel builds; the workers have two vCPUs.  Also, use the build-*
>>    targets rather than the ones which expand to dist-*.
>>  * Shrink the dependency list further.  build-essential covers make and gcc,
>>    while bridge-utils and iproute2 are runtime dependencies not build
>>    dependencies.  Alter bzip2 to libbz2-dev.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks,

~Andrew

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

[Roger Pau Monné]

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.

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

Thanks, Roger.

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

[Jan Beulich]

On 21.02.2022 11:02, 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.
> 
> While at it, fix style issues.  Rearrange ioreq_server_alloc_rangesets() to
> use a tabulated switch statement, and not to have a trailing space in the
> rangeset name for an unknown range type.
> 
> Coverity-ID: 1472735
> Coverity-ID: 1500265
> Fixes: 780e918a2e54 ("add an implentation of asprintf() for xen")
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

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

[Andrew Cooper]

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