From: Peter Krempa <pkrempa@redhat.com>

Use 'virFileReadAllQuiet' since the function doesn't want to report
errors on other code paths.

The function also assumed that the file which it reads always 7 bytes
isn't true at least in the test suite. This didn't cause a problem
because the test data had strings 6 bytes long so it didn't cause a
write beyond the end of the buffer.

Clear the newline by using strchrnul instead to find it rather than
assuming where it is.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/util/virpci.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 2e32ed17ff..48cdffe3d4 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1760,19 +1760,20 @@ virPCIDeviceReadID(virPCIDevice *dev, const char *id_name)
 {
     g_autofree char *path = NULL;
     g_autofree char *id_str = NULL;
+    int len;

     path = virPCIFile(dev->name, id_name);

     /* ID string is '0xNNNN\n' ... i.e. 7 bytes */
-    if (virFileReadAll(path, 7, &id_str) < 0)
+    if ((len = virFileReadAllQuiet(path, 7, &id_str)) < 0)
         return NULL;

-    /* Check for 0x suffix */
+    /* Check for 0x prefix */
     if (id_str[0] != '0' || id_str[1] != 'x')
         return NULL;

-    /* Chop off the newline; we know the string is 7 bytes */
-    id_str[6] = '\0';
+    /* Chop off the newline */
+    *(strchrnul(id_str, '\n')) = '\0';

     return g_steal_pointer(&id_str);
 }
-- 
2.53.0

On Fri, Mar 27, 2026 at 10:54:42 +0100, Peter Krempa via Devel wrote:
> From: Peter Krempa <pkrempa@redhat.com>
> 
> Use 'virFileReadAllQuiet' since the function doesn't want to report
> errors on other code paths.
> 
> The function also assumed that the file which it reads always 7 bytes
> isn't true at least in the test suite. This didn't cause a problem
> because the test data had strings 6 bytes long so it didn't cause a
> write beyond the end of the buffer.
> 
> Clear the newline by using strchrnul instead to find it rather than
> assuming where it is.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/util/virpci.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/src/util/virpci.c b/src/util/virpci.c
> index 2e32ed17ff..48cdffe3d4 100644
> --- a/src/util/virpci.c
> +++ b/src/util/virpci.c
> @@ -1760,19 +1760,20 @@ virPCIDeviceReadID(virPCIDevice *dev, const char *id_name)
>  {
>      g_autofree char *path = NULL;
>      g_autofree char *id_str = NULL;
> +    int len;
> 
>      path = virPCIFile(dev->name, id_name);
> 
>      /* ID string is '0xNNNN\n' ... i.e. 7 bytes */
> -    if (virFileReadAll(path, 7, &id_str) < 0)
> +    if ((len = virFileReadAllQuiet(path, 7, &id_str)) < 0)
>          return NULL;
> 
> -    /* Check for 0x suffix */
> +    /* Check for 0x prefix */
>      if (id_str[0] != '0' || id_str[1] != 'x')
>          return NULL;
> 
> -    /* Chop off the newline; we know the string is 7 bytes */
> -    id_str[6] = '\0';
> +    /* Chop off the newline */
> +    *(strchrnul(id_str, '\n')) = '\0';

Gah, this is _GNU_SOURCE, thus OSX and mingw don't like it.

On Fri, Mar 27, 2026 at 11:32:47 +0100, Peter Krempa via Devel wrote:
> On Fri, Mar 27, 2026 at 10:54:42 +0100, Peter Krempa via Devel wrote:
> > From: Peter Krempa <pkrempa@redhat.com>
> > 
> > Use 'virFileReadAllQuiet' since the function doesn't want to report
> > errors on other code paths.
> > 
> > The function also assumed that the file which it reads always 7 bytes
> > isn't true at least in the test suite. This didn't cause a problem
> > because the test data had strings 6 bytes long so it didn't cause a
> > write beyond the end of the buffer.
> > 
> > Clear the newline by using strchrnul instead to find it rather than
> > assuming where it is.
> > 
> > Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> > ---
> >  src/util/virpci.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/util/virpci.c b/src/util/virpci.c
> > index 2e32ed17ff..48cdffe3d4 100644
> > --- a/src/util/virpci.c
> > +++ b/src/util/virpci.c
> > @@ -1760,19 +1760,20 @@ virPCIDeviceReadID(virPCIDevice *dev, const char *id_name)
> >  {
> >      g_autofree char *path = NULL;
> >      g_autofree char *id_str = NULL;
> > +    int len;
> > 
> >      path = virPCIFile(dev->name, id_name);
> > 
> >      /* ID string is '0xNNNN\n' ... i.e. 7 bytes */
> > -    if (virFileReadAll(path, 7, &id_str) < 0)
> > +    if ((len = virFileReadAllQuiet(path, 7, &id_str)) < 0)
> >          return NULL;
> > 
> > -    /* Check for 0x suffix */
> > +    /* Check for 0x prefix */
> >      if (id_str[0] != '0' || id_str[1] != 'x')
> >          return NULL;
> > 
> > -    /* Chop off the newline; we know the string is 7 bytes */
> > -    id_str[6] = '\0';
> > +    /* Chop off the newline */
> > +    *(strchrnul(id_str, '\n')) = '\0';
> 
> Gah, this is _GNU_SOURCE, thus OSX and mingw don't like it.
> 

Consider this replaced with 'virStringTrimOptionalNewline(id_str);'