On 11/29/12 20:04, Eric Blake wrote:
This patch adds macros to help retrieve configuration values from
qemu
driver's configuration. Some configuration options are grouped
together in the process.
---
src/qemu/qemu_conf.c | 303
+++++++++++++--------------------------------------
1 file changed, 73 insertions(+), 230 deletions(-)
Always fun to see refactoring that reduces code size.
Indeed!
+#define GET_VALUE_LONG(NAME, VAR) p = virConfGetValue(conf, NAME);
\
This feels like a long line; I'd format it slightly differently:
#define GET_VALUE_LONG(NAME, VAR) \
p = virConfGetValue(conf, NAME); \
...
+ CHECK_TYPE(NAME, VIR_CONF_LONG);
\
+ if (p) VAR = p->l;
As long as you are refactoring, can you please split this line
into two:
if (p) \
VAR = p->l
per our coding conventions?
Ah yeah ... I was doing the conversions in a kind of braindead way.
+
+#define GET_VALUE_STR(NAME, VAR) p = virConfGetValue(conf, NAME);
\
Again, this feels like a lot of indentation; starting the first
line of the macro body after a continuation will get rid of a
lot of this whitespace.
+ /* increasing the value by 1 makes all the loops going through
+ the bitmap (i = remotePortMin; i < remotePortMax; i++), work as
+ expected. */
+ driver->remotePortMax += 1;
Why ' += 1' instead of the shorter '++'?
It was pre-existing but I'm touching the line anyways so I'll change that.
+ GET_VALUE_LONG("max_queued", driver->max_queued);
+ GET_VALUE_LONG("keepalive_interval", driver->keepAliveInterval);
+ GET_VALUE_LONG("keepalive_count", driver->keepAliveCount);
+ GET_VALUE_LONG("seccomp_sandbox", driver->seccompSandbox);
virConfFree(conf);
return 0;
Should we add
#undef GET_VALUE_LONG
#undef GET_VALUE_STRING
at the end, since our macros are merely local helpers for this
function?
Definitely.
ACK with nits addressed.
Pushed, thanks.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list