"Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote:
> On Tue, Jan 27, 2009 at 04:59:20PM +0100, Jim Meyering wrote:
>> Here's the big one:
>> >From 099536470ae2cbe9503ed471d391e403fb2587a4 Mon Sep 17 00:00:00 2001
>> From: Jim Meyering <meyering@xxxxxxxxxx>
>> Date: Tue, 27 Jan 2009 12:20:06 +0100
>> Subject: [PATCH 1/3] error-reporting calls using VIR_ERR_NO_MEMORY: use virReportOOMError instead
>>
>> * src/uml_conf.c (VIR_FROM_THIS): Define to VIR_FROM_UML.
>> * src/xs_internal.c (VIR_FROM_THIS): Define to VIR_FROM_XEN.
>> * src/xml.c (VIR_FROM_THIS): Define to VIR_FROM_XML.
>> * src/stats_linux.c (VIR_FROM_THIS): Define to VIR_FROM_STATS_LINUX.
>> * src/datatypes.c (VIR_FROM_THIS): Define to VIR_FROM_NONE.
>> * src/lxc_conf.c (VIR_FROM_THIS): Define to VIR_FROM_LXC.
>> * src/libvirt.c (VIR_FROM_THIS): Define to VIR_FROM_NONE.
>> * src/node_device_conf.c (VIR_FROM_THIS): Define to VIR_FROM_NODEDEV.
>> * src/openvz_conf.c (VIR_FROM_THIS): Define to VIR_FROM_OPENVZ.
>> * src/openvz_driver.c (VIR_FROM_THIS): Define to VIR_FROM_OPENVZ.
>> * src/conf.c (VIR_FROM_THIS): Define to VIR_FROM_CONF.
>> Note: this loses config_filename:config_lineno diagnostics,
>> but that's ok.
>> * src/node_device.c (VIR_FROM_THIS): Define to VIR_FROM_NODEDEV.
>> * src/sexpr.c (VIR_FROM_THIS): Define to VIR_FROM_SEXPR.
>
> ACK.
>
> After a quick run through the patch I don't see any problems hiding
> there - well at least none which weren't already present.
...
> We should add a Makefile.maint rule to prohibit use of VIR_ERR_NO_MEMORY
> in any file except virterror.c
There were a few more to remove, so here are two patches.
First, an incremental removing the remaining uses of VIR_ERR_NO_MEMORY.
Then a patch adding a syntax-check rule to prevent reintroduction.
Note that the Makefile.maint patch isn't usable as-is;
it requires a preceding patch to sync from coreutils's maint.mk.
And once I did that, a few more syntax-check failures were triggered,
and so I disabled the corresponding rules (temporarily) in another patch.
Both coming up.
FYI, here are the exceptions in .c/.h files:
include/libvirt/virterror.h-typedef enum {
include/libvirt/virterror.h- VIR_ERR_OK = 0,
include/libvirt/virterror.h- VIR_ERR_INTERNAL_ERROR, /* internal error */
include/libvirt/virterror.h: VIR_ERR_NO_MEMORY, /* memory allocation failure */
include/libvirt/virterror.h- VIR_ERR_NO_SUPPORT, /* no support for this function */
include/libvirt/virterror.h- VIR_ERR_UNKNOWN_HOST,/* could not resolve hostname */
include/libvirt/virterror.h- VIR_ERR_NO_CONNECT, /* can't connect to hypervisor */
--
qemud/remote.c-remoteDispatchOOMError (remote_error *rerr)
qemud/remote.c-{
qemud/remote.c- remoteDispatchStringError(rerr,
qemud/remote.c: VIR_ERR_NO_MEMORY,
qemud/remote.c- NULL);
qemud/remote.c-}
qemud/remote.c-
--
src/virterror.c- else
src/virterror.c- errmsg = _("internal error");
src/virterror.c- break;
src/virterror.c: case VIR_ERR_NO_MEMORY:
src/virterror.c- errmsg = _("out of memory");
src/virterror.c- break;
src/virterror.c- case VIR_ERR_NO_SUPPORT:
--
src/virterror.c-{
src/virterror.c- const char *virerr;
src/virterror.c-
src/virterror.c: virerr = virErrorMsg(VIR_ERR_NO_MEMORY, NULL);
src/virterror.c: virRaiseError(conn, NULL, NULL, domcode, VIR_ERR_NO_MEMORY, VIR_ERR_ERROR,
src/virterror.c- virerr, NULL, NULL, -1, -1, virerr, NULL);
src/virterror.c-}
>From afa312df336037fcfc166939f3aa37513da4e41d Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Tue, 27 Jan 2009 21:37:13 +0100
Subject: [PATCH 4/8] FIXME: to-be-squashed: VIR_ERR_NO_MEMORY -> virReportOOMError
---
src/remote_internal.c | 8 ++------
src/sexpr.c | 3 +--
src/storage_backend_fs.c | 7 +++----
src/storage_backend_logical.c | 4 ++--
4 files changed, 8 insertions(+), 14 deletions(-)
diff --git a/src/remote_internal.c b/src/remote_internal.c
index 5c2e705..0fb5d87 100644
--- a/src/remote_internal.c
+++ b/src/remote_internal.c
@@ -4957,10 +4957,7 @@ static char *addrToString(struct sockaddr_storage *sa, socklen_t salen)
}
if (VIR_ALLOC_N(addr, strlen(host) + 1 + strlen(port) + 1) < 0) {
- virRaiseError (NULL, NULL, NULL, VIR_FROM_REMOTE,
- VIR_ERR_NO_MEMORY, VIR_ERR_ERROR,
- NULL, NULL, NULL, 0, 0,
- "address");
+ virReportOOMError (NULL);
return NULL;
}
@@ -6315,8 +6312,7 @@ call (virConnectPtr conn, struct private_data *priv,
ret_filter, ret);
if (!thiscall) {
- error (flags & REMOTE_CALL_IN_OPEN ? NULL : conn,
- VIR_ERR_NO_MEMORY, NULL);
+ virReportOOMError (flags & REMOTE_CALL_IN_OPEN ? NULL : conn);
return -1;
}
diff --git a/src/sexpr.c b/src/sexpr.c
index 7450c79..bc82d1f 100644
--- a/src/sexpr.c
+++ b/src/sexpr.c
@@ -361,8 +361,7 @@ _string2sexpr(const char *buffer, size_t * end)
ret->u.value = strndup(start, ptr - start);
if (ret->u.value == NULL) {
- virSexprError(VIR_ERR_NO_MEMORY,
- "%s", _("failed to copy a string"));
+ virReportOOMError(NULL);
goto error;
}
}
diff --git a/src/storage_backend_fs.c b/src/storage_backend_fs.c
index 6fd9f8f..345dc40 100644
--- a/src/storage_backend_fs.c
+++ b/src/storage_backend_fs.c
@@ -196,7 +196,7 @@ qcowXGetBackingStore(virConnectPtr conn,
if (size + 1 == 0)
return BACKING_STORE_INVALID;
if (VIR_ALLOC_N(*res, size + 1) < 0) {
- virStorageReportError(conn, VIR_ERR_NO_MEMORY, _("backing store path"));
+ virReportOOMError(conn);
return BACKING_STORE_ERROR;
}
memcpy(*res, buf + offset, size);
@@ -237,7 +237,7 @@ vmdk4GetBackingStore(virConnectPtr conn,
*end = '\0';
*res = strdup(start);
if (*res == NULL) {
- virStorageReportError(conn, VIR_ERR_NO_MEMORY, _("backing store path"));
+ virReportOOMError(conn);
return BACKING_STORE_ERROR;
}
return BACKING_STORE_OK;
@@ -395,8 +395,7 @@ static int virStorageBackendProbeTarget(virConnectPtr conn,
= absolutePathFromBaseFile(target->path, base);
VIR_FREE(base);
if (*backingStore == NULL) {
- virStorageReportError(conn, VIR_ERR_NO_MEMORY,
- _("backing store path"));
+ virReportOOMError(conn);
return -1;
}
}
diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
index 857262d..702a191 100644
--- a/src/storage_backend_logical.c
+++ b/src/storage_backend_logical.c
@@ -127,7 +127,7 @@ virStorageBackendLogicalMakeVol(virConnectPtr conn,
if (vol->key == NULL &&
(vol->key = strdup(groups[2])) == NULL) {
- virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("volume"));
+ virReportOOMError(conn);
return -1;
}
@@ -144,7 +144,7 @@ virStorageBackendLogicalMakeVol(virConnectPtr conn,
if ((vol->source.extents[vol->source.nextent].path =
strdup(groups[3])) == NULL) {
- virStorageReportError(conn, VIR_ERR_NO_MEMORY, "%s", _("extents"));
+ virReportOOMError(conn);
return -1;
}
--
1.6.1.1.374.g0d9d7
>From 3d47ae92e1148c85ba0e4a22848b250681c13a22 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@xxxxxxxxxx>
Date: Wed, 28 Jan 2009 16:49:58 +0100
Subject: [PATCH 8/8] prohibit new uses of VIR_ERR_NO_MEMORY
* Makefile.maint (sc_prohibit_VIR_ERR_NO_MEMORY): New rule.
* .x-sc_prohibit_VIR_ERR_NO_MEMORY: New file: exceptions.
---
.x-sc_prohibit_VIR_ERR_NO_MEMORY | 8 ++++++++
Makefile.maint | 6 ++++++
2 files changed, 14 insertions(+), 0 deletions(-)
create mode 100644 .x-sc_prohibit_VIR_ERR_NO_MEMORY
diff --git a/.x-sc_prohibit_VIR_ERR_NO_MEMORY b/.x-sc_prohibit_VIR_ERR_NO_MEMORY
new file mode 100644
index 0000000..efb2012
--- /dev/null
+++ b/.x-sc_prohibit_VIR_ERR_NO_MEMORY
@@ -0,0 +1,8 @@
+ChangeLog
+docs/devhelp/libvirt-virterror.html
+docs/html/libvirt-virterror.html
+docs/libvirt-api.xml
+docs/libvirt-refs.xml
+include/libvirt/virterror.h
+qemud/remote.c
+src/virterror.c
diff --git a/Makefile.maint b/Makefile.maint
index c50d100..97f2526 100644
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -112,6 +112,11 @@ sc_prohibit_asprintf:
msg='use virAsprintf, not a'sprintf \
$(_prohibit_regexp)
+sc_prohibit_VIR_ERR_NO_MEMORY:
+ @re='\<V''IR_ERR_NO_MEMORY\>' \
+ msg='use virReportOOMError, not V'IR_ERR_NO_MEMORY \
+ $(_prohibit_regexp)
+
sc_prohibit_nonreentrant:
@fail=0 ; \
for i in $(NON_REENTRANT) ; \
--
1.6.1.1.374.g0d9d7
--
Libvir-list mailing list
Libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list