Re: [PATCH 04/10] remote_driver: Expose EXTRACT_URI_ARG_* macros
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(a)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; \
+ }