[PATCH 0/4] tools/libs: don't set errno to negative values

[PATCH 0/4] tools/libs: don't set errno to negative values

[Juergen Gross]

There are a few places in the libs where errno is set to a negative
value. Fix those.

Juergen Gross (4):
  tools/libs/evtchn: don't set errno to negative values
  tools/libs/ctrl: don't set errno to a negative value
  tools/libs/guest: don't set errno to a negative value
  tools/libs/light: don't set errno to a negative value

 tools/libs/ctrl/xc_domain.c    | 4 +---
 tools/libs/evtchn/freebsd.c    | 2 +-
 tools/libs/evtchn/minios.c     | 2 +-
 tools/libs/evtchn/netbsd.c     | 2 +-
 tools/libs/evtchn/solaris.c    | 2 +-
 tools/libs/guest/xg_dom_core.c | 2 +-
 tools/libs/light/libxl_linux.c | 2 +-
 7 files changed, 7 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH 1/4] tools/libs/evtchn: don't set errno to negative values

[Juergen Gross]

Setting errno to a negative value makes no sense.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libs/evtchn/freebsd.c | 2 +-
 tools/libs/evtchn/minios.c  | 2 +-
 tools/libs/evtchn/netbsd.c  | 2 +-
 tools/libs/evtchn/solaris.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/libs/evtchn/freebsd.c b/tools/libs/evtchn/freebsd.c
index c4d075350b..422836f1a1 100644
--- a/tools/libs/evtchn/freebsd.c
+++ b/tools/libs/evtchn/freebsd.c
@@ -58,7 +58,7 @@ int osdep_evtchn_close(xenevtchn_handle *xce)
 
 int osdep_evtchn_restrict(xenevtchn_handle *xce, domid_t domid)
 {
-    errno = -EOPNOTSUPP;
+    errno = EOPNOTSUPP;
 
     return -1;
 }
diff --git a/tools/libs/evtchn/minios.c b/tools/libs/evtchn/minios.c
index 65cfccfd09..8ff46de884 100644
--- a/tools/libs/evtchn/minios.c
+++ b/tools/libs/evtchn/minios.c
@@ -143,7 +143,7 @@ int osdep_evtchn_close(xenevtchn_handle *xce)
 
 int osdep_evtchn_restrict(xenevtchn_handle *xce, domid_t domid)
 {
-    errno = -EOPNOTSUPP;
+    errno = EOPNOTSUPP;
 
     return -1;
 }
diff --git a/tools/libs/evtchn/netbsd.c b/tools/libs/evtchn/netbsd.c
index 0b223c4beb..2de9cefc67 100644
--- a/tools/libs/evtchn/netbsd.c
+++ b/tools/libs/evtchn/netbsd.c
@@ -53,7 +53,7 @@ int osdep_evtchn_close(xenevtchn_handle *xce)
 
 int osdep_evtchn_restrict(xenevtchn_handle *xce, domid_t domid)
 {
-    errno = -EOPNOTSUPP;
+    errno = EOPNOTSUPP;
 
     return -1;
 }
diff --git a/tools/libs/evtchn/solaris.c b/tools/libs/evtchn/solaris.c
index 7fef88a73e..375a5f9b4f 100644
--- a/tools/libs/evtchn/solaris.c
+++ b/tools/libs/evtchn/solaris.c
@@ -53,7 +53,7 @@ int osdep_evtchn_close(xenevtchn_handle *xce)
 
 int osdep_evtchn_restrict(xenevtchn_handle *xce, domid_t domid)
 {
-    errno = -EOPNOTSUPP;
+    errno = EOPNOTSUPP;
     return -1;
 }
 
-- 
2.34.1

Re: [PATCH 1/4] tools/libs/evtchn: don't set errno to negative values

[Andrew Cooper]

On 20/04/2022 08:31, Juergen Gross wrote:
> Setting errno to a negative value makes no sense.

Fixes: 6b6500b3cbaa

> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

[PATCH 2/4] tools/libs/ctrl: don't set errno to a negative value

[Juergen Gross]

The claimed reason for setting errno to -1 is wrong. On x86
xc_domain_pod_target() will set errno to a sane value in the error
case.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libs/ctrl/xc_domain.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c
index ef62f66009..71608c00e9 100644
--- a/tools/libs/ctrl/xc_domain.c
+++ b/tools/libs/ctrl/xc_domain.c
@@ -1293,9 +1293,7 @@ int xc_domain_get_pod_target(xc_interface *xch,
                              uint64_t *pod_cache_pages,
                              uint64_t *pod_entries)
 {
-    /* On x86 (above) xc_domain_pod_target will incorrectly return -1
-     * with errno==-1 on error. Do the same for least surprise. */
-    errno = -1;
+    errno = EOPNOTSUPP;
     return -1;
 }
 #endif
-- 
2.34.1

Re: [PATCH 2/4] tools/libs/ctrl: don't set errno to a negative value

[Andrew Cooper]

On 20/04/2022 08:31, Juergen Gross wrote:
> The claimed reason for setting errno to -1 is wrong. On x86
> xc_domain_pod_target() will set errno to a sane value in the error
> case.

Fixes: ff1745d5882b

> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

[PATCH 3/4] tools/libs/guest: don't set errno to a negative value

[Juergen Gross]

Setting errno to a negative error value makes no sense.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libs/guest/xg_dom_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libs/guest/xg_dom_core.c b/tools/libs/guest/xg_dom_core.c
index c17cf9f712..c4f4e7f3e2 100644
--- a/tools/libs/guest/xg_dom_core.c
+++ b/tools/libs/guest/xg_dom_core.c
@@ -855,7 +855,7 @@ int xc_dom_devicetree_file(struct xc_dom_image *dom, const char *filename)
         return -1;
     return 0;
 #else
-    errno = -EINVAL;
+    errno = EINVAL;
     return -1;
 #endif
 }
-- 
2.34.1

Re: [PATCH 3/4] tools/libs/guest: don't set errno to a negative value

[Andrew Cooper]

On 20/04/2022 08:31, Juergen Gross wrote:
> Setting errno to a negative error value makes no sense.

Fixes: cb99a64029c9d

> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

[PATCH 4/4] tools/libs/light: don't set errno to a negative value

[Juergen Gross]

Setting errno to a negative value makes no sense.

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 tools/libs/light/libxl_linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libs/light/libxl_linux.c b/tools/libs/light/libxl_linux.c
index 8d62dfd255..27f2bce718 100644
--- a/tools/libs/light/libxl_linux.c
+++ b/tools/libs/light/libxl_linux.c
@@ -288,7 +288,7 @@ int libxl__pci_topology_init(libxl__gc *gc,
         if (i == num_devs) {
             LOG(ERROR, "Too many devices");
             err = ERROR_FAIL;
-            errno = -ENOSPC;
+            errno = ENOSPC;
             goto out;
         }
 
-- 
2.34.1

Re: [PATCH 4/4] tools/libs/light: don't set errno to a negative value

[Andrew Cooper]

On 20/04/2022 08:31, Juergen Gross wrote:
> Setting errno to a negative value makes no sense.

Fixes: e78e8b9bb649b

> Signed-off-by: Juergen Gross <jgross@suse.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Re: [PATCH 4/4] tools/libs/light: don't set errno to a negative value

[Juergen Gross]

[-- Attachment #1.1.1: Type: text/plain, Size: 553 bytes --]

On 22.04.22 16:52, Andrew Cooper wrote:
> On 20/04/2022 08:31, Juergen Gross wrote:
>> Setting errno to a negative value makes no sense.
> 
> Fixes: e78e8b9bb649b
> 
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

Do you want me to send the patches with the Fixes tags added?

TBH I didn't bother to do the research for those, as the wrong values
don't seem to cause any harm today. I just stumbled over the issues
and thought it would be a good idea to fix those.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]

Re: [PATCH 4/4] tools/libs/light: don't set errno to a negative value

[Andrew Cooper]

On 22/04/2022 15:55, Juergen Gross wrote:
> On 22.04.22 16:52, Andrew Cooper wrote:
>> On 20/04/2022 08:31, Juergen Gross wrote:
>>> Setting errno to a negative value makes no sense.
>>
>> Fixes: e78e8b9bb649b
>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>
>> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Do you want me to send the patches with the Fixes tags added?
>
> TBH I didn't bother to do the research for those, as the wrong values
> don't seem to cause any harm today. I just stumbled over the issues
> and thought it would be a good idea to fix those.

Given that there's nothing else to adjust, I was thinking of just fixing
them on commit.

But we should backport these to all appropriate releases (cc Jan).

~Andrew