On Mon, Feb 06, 2023 at 10:16:52 +0100, Michal Privoznik wrote:
Almost in all places where an URI is parsed we look for additional argument(s). The remote driver's parsing uses two macros EXTRACT_URI_ARG_STR() and EXTRACT_URI_ARG_BOOL() for that purpose. Expose these so that other places can be rewritten using those macros.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- po/POTFILES | 1 + src/remote/remote_driver.c | 58 +++++++++++--------------------------- src/util/viruri.h | 23 +++++++++++++++ 3 files changed, 40 insertions(+), 42 deletions(-)
[...]
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 6a226999df..c41d5b414f 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -693,30 +693,6 @@ remoteConnectSupportsFeatureUnlocked(virConnectPtr conn, return rc != -1 && ret.supported; }
-/* helper macro to ease extraction of arguments from the URI */ -#define EXTRACT_URI_ARG_STR(ARG_NAME, ARG_VAR) \ - if (STRCASEEQ(var->name, ARG_NAME)) { \ - VIR_FREE(ARG_VAR); \ - ARG_VAR = g_strdup(var->value); \ - var->ignore = 1; \ - continue; \ - }
This style of macros, which apart from arguments also accesses variables directly from the place where it's expanded or influence control flow ...
@@ -796,23 +772,23 @@ doRemoteOpen(virConnectPtr conn, if (conn->uri) { for (i = 0; i < conn->uri->paramsCount; i++) { virURIParam *var = &conn->uri->params[i]; - EXTRACT_URI_ARG_STR("name", name); - EXTRACT_URI_ARG_STR("command", command); - EXTRACT_URI_ARG_STR("socket", sockname); - EXTRACT_URI_ARG_STR("auth", authtype);
... are really acceptable only in ad-hoc situations as here ...
@@ -1206,8 +1182,6 @@ doRemoteOpen(virConnectPtr conn, VIR_FREE(priv->hostname); return VIR_DRV_OPEN_ERROR; } -#undef EXTRACT_URI_ARG_STR -#undef EXTRACT_URI_ARG_BOOL
... where we dispose of it right away.
static struct private_data * remoteAllocPrivateData(void) diff --git a/src/util/viruri.h b/src/util/viruri.h index 4f27fa26d2..0e4176c037 100644 --- a/src/util/viruri.h +++ b/src/util/viruri.h @@ -62,3 +62,26 @@ const char *virURIGetParam(virURI *uri, const char *name); bool virURICheckUnixSocket(virURI *uri);
#define VIR_URI_SERVER(uri) ((uri) && (uri)->server ? (uri)->server : "localhost") + +/* helper macros to ease extraction of arguments from the URI */ +#define VIR_EXTRACT_URI_ARG_STR(ARG_NAME, ARG_VAR) \
Exporting it as such is not acceptable. Less so with non-existent docs outlining the "side effects" not visible from the place where it's called. I suggest you turn it into a function (keeping proper naming scheme) such as: int virURIParamExtractStr(virURIParam *par, const char *name, char **val) { if (STRCASEEQ(par->name, name)) { g_free(*val); *val = g_strdup(par->value); par->ignore = true; return 1; } return 0; } Alternatively return a bool. And use it as: if (virURIParamExtractStr(var, "name", &name) == 1) continue;
+ if (STRCASEEQ(var->name, ARG_NAME)) { \ + g_free(ARG_VAR); \ + ARG_VAR = g_strdup(var->value); \ + var->ignore = 1; \ + continue; \ + }