On 2/1/21 7:27 AM, Laine Stump wrote:
This may be pointless, but I noticed it and it was bugging me.
virGetConnectNetwork() calls
virGetConnectGeneric(), which calls
virConnecCacheInitialize(), which is actually a call (only once) to
virConnectCacheOnceInit() which calls
virThreadLocalInit() several times, which calls
pthread_key_create()
If pthread_key_create() fails, it (of course) doesn't log an error
(because it's not a part of libvirt), nor does any other function on
the call chain all the way up to virGetConnectNetwork(). But none of
the callers of virGetConnectNetwork() log an error either, so it is
possible that an API could fail due to virGetConnectNetwork() failing,
but would only log "an error was encountered, but the cause is
unknown. Deal with it." (paraphrasing).
In all likelyhood, virConnectCacheOnceInit() is going to be called at
some earlier time, and almost certainly pthread_key_create() will
never fail (and if it does, the user will have *much* bigger problems
than an obtuse error message from libvirt). So I don't know if there's
any value at all in pushing this. Just throwing it out there...
Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
---
src/driver.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/driver.c b/src/driver.c
index e005a89d57..1dae7cf284 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -116,18 +116,18 @@ virThreadLocal connectStorage;
static int
virConnectCacheOnceInit(void)
{
- if (virThreadLocalInit(&connectInterface, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectNetwork, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectNWFilter, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectNodeDev, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectSecret, NULL) < 0)
- return -1;
- if (virThreadLocalInit(&connectStorage, NULL) < 0)
+ if (virThreadLocalInit(&connectInterface, NULL) < 0
+ || virThreadLocalInit(&connectNetwork, NULL) < 0
+ || virThreadLocalInit(&connectNWFilter, NULL) < 0
+ || virThreadLocalInit(&connectNodeDev, NULL) < 0
+ || virThreadLocalInit(&connectSecret, NULL) < 0
+ || virThreadLocalInit(&connectStorage, NULL) < 0) {
I think our preferred way of breaking these checks is to put '||' at the
end of each line rather than at the beginning. It it isn't then it
should be :-)
+
Maybe drop this empty line?
+ virReportSystemError(errno, "%s",
+ _("Unable to initialize thread local variable"));
return -1;
+ }
+
return 0;
}
Michal
