7:36 a.m.
Without this patch, building without HAVE_AVAHI fails due to an unguarded
use of mdns_name. Rather than adding yet another #ifdef..#endif (ugly
and less maintainable), I've chosen to un-ifdef the declarations and
a few uses. The more code that we compile and use unconditionally,
the fewer surprises we'll encounter down the road.
Mon Dec 3 14:24:20 CET 2007 Jim Meyering <meyering(a)redhat.com>
Avoid compile failure when HAVE_AVAHI is not defined.
* qemud/qemud.c (remoteReadConfigFile): Remove some of the
"#ifdef HAVE_AVAHI" guards around uses of mdns_name and mdns_adv.
diff --git a/qemud/qemud.c b/qemud/qemud.c
index 9904e4a..f88ed42 100644
--- a/qemud/qemud.c
+++ b/qemud/qemud.c
@@ -77,10 +77,8 @@ static gid_t unix_sock_gid = 0; /* Only root by default */
static int unix_sock_rw_mask = 0700; /* Allow user only */
static int unix_sock_ro_mask = 0777; /* Allow world */
-#ifdef HAVE_AVAHI
static int mdns_adv = 1;
static char *mdns_name = NULL;
-#endif
static int tls_no_verify_certificate = 0;
static int tls_no_verify_address = 0;
@@ -1696,10 +1694,8 @@ remoteReadConfigFile (const char *filename)
unix_sock_rw_perms = NULL;
}
-#ifdef HAVE_AVAHI
GET_CONF_INT (conf, filename, mdns_adv);
GET_CONF_STR (conf, filename, mdns_name);
-#endif
GET_CONF_INT (conf, filename, tls_no_verify_certificate);
GET_CONF_INT (conf, filename, tls_no_verify_address);
--
1.5.3.7.949.g2221a6
8:43 a.m.
Jim Meyering wrote:
Without this patch, building without HAVE_AVAHI fails due to an
unguarded
use of mdns_name. Rather than adding yet another #ifdef..#endif (ugly
and less maintainable), I've chosen to un-ifdef the declarations and
a few uses. The more code that we compile and use unconditionally,
the fewer surprises we'll encounter down the road.
Strangely enough I didn't hit this problem when building on Cygwin, and
I'm pretty sure they don't have avahi?
My only reservation is what happens if mdns_* configuration is in
libvirtd.conf, but there is no support in the binary. Should we print
out an error message or a warning?
General principles though +1
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
8:56 a.m.
"Richard W.M. Jones" <rjones(a)redhat.com> wrote:
Jim Meyering wrote:
> Without this patch, building without HAVE_AVAHI fails due to an unguarded
> use of mdns_name. Rather than adding yet another #ifdef..#endif (ugly
> and less maintainable), I've chosen to un-ifdef the declarations and
> a few uses. The more code that we compile and use unconditionally,
> the fewer surprises we'll encounter down the road.
Strangely enough I didn't hit this problem when building on Cygwin,
and I'm pretty sure they don't have avahi?
That file has changed recently. I've just looked it up
and see that it's the "free (mdns_name);" that I added.
My only reservation is what happens if mdns_* configuration is in
libvirtd.conf, but there is no support in the binary. Should we print
out an error message or a warning?
IMHO, no, but it wouldn't hurt.
The current policy is to ignore config-file errors
for compiled-out features.
If we do change the policy, we should do it across the board.
However, it doesn't seem worth the trouble right now.
Why spend effort on broken or poorly-configured systems?
8:59 a.m.
On Mon, Dec 03, 2007 at 02:43:30PM +0000, Richard W.M. Jones wrote:
Jim Meyering wrote:
>Without this patch, building without HAVE_AVAHI fails due to an unguarded
>use of mdns_name. Rather than adding yet another #ifdef..#endif (ugly
>and less maintainable), I've chosen to un-ifdef the declarations and
>a few uses. The more code that we compile and use unconditionally,
>the fewer surprises we'll encounter down the road.
Strangely enough I didn't hit this problem when building on Cygwin, and
I'm pretty sure they don't have avahi?
My only reservation is what happens if mdns_* configuration is in
libvirtd.conf, but there is no support in the binary. Should we print
out an error message or a warning?
Its not worth the bother IMHO. You're adding extra code to cover one tiny
niche case which will rarely ever be needed since all modern Linux have
avahi.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
10:08 a.m.
Richard W.M. Jones wrote:
Strangely enough I didn't hit this problem when building on
Cygwin, and
I'm pretty sure they don't have avahi?
Update: yes, I'm now hitting this problem, and your patch fixes it.
Rich.
--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom. Registered in
England and Wales under Company Registration No. 03798903
8:56 a.m.
On Mon, Dec 03, 2007 at 02:36:17PM +0100, Jim Meyering wrote:
Without this patch, building without HAVE_AVAHI fails due to an
unguarded
use of mdns_name. Rather than adding yet another #ifdef..#endif (ugly
and less maintainable), I've chosen to un-ifdef the declarations and
a few uses. The more code that we compile and use unconditionally,
the fewer surprises we'll encounter down the road.
I disagree - adding merely one extra HAVE_AVAHI in the cleanup code for
that function won't hurt maintainability.
I find it more confusing/suprising when I find variables which are there
but not used in any code like mdns_adv vars are with this patch.
Dan.
--
|=- Red Hat, Engineering, Emerging Technologies, Boston. +1 978 392 2496 -=|
|=- Perl modules: http://search.cpan.org/~danberr/ -=|
|=- Projects: http://freshmeat.net/~danielpb/ -=|
|=- GnuPG: 7D3B9505 F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 -=|
10:49 a.m.
On Mon, Dec 03, 2007 at 02:36:17PM +0100, Jim Meyering wrote:
Without this patch, building without HAVE_AVAHI fails due to an
unguarded
use of mdns_name. Rather than adding yet another #ifdef..#endif (ugly
and less maintainable), I've chosen to un-ifdef the declarations and
a few uses. The more code that we compile and use unconditionally,
the fewer surprises we'll encounter down the road.
Mon Dec 3 14:24:20 CET 2007 Jim Meyering <meyering(a)redhat.com>
Avoid compile failure when HAVE_AVAHI is not defined.
* qemud/qemud.c (remoteReadConfigFile): Remove some of the
"#ifdef HAVE_AVAHI" guards around uses of mdns_name and mdns_adv.
I tend to disagree. If we know at compile time that some code should
not be included, then we should not do it. Sometimes there are better
solutions to this problem than an explosion of #define/#endif but
it's the most common way unfortunately. What we can do is try to isolate
into function specific (or even better module specific) the parts which
are really dependant on a single feature.
To me usually a single level of define in a function is relatively
maintainable long-term, things like
int operation_foo(...) {
....
#ifdef HAVE_BAR
...
do_some_bar_stuff()
...
#else
do_something_else()
#endif
....
}
looks okay to me. The problem arise when we start to see imbricated
#ifdef/endif blocks, IMHO this usually means that the function should be
broken into smaller chunks which addresses more specific subpart of the
task. A bit like when indenting starts to force you to write code which
doesn't fit in 80 columns, same kind of problem, same kind of solutions.
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
veillard(a)redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
1:47 p.m.
Jim Meyering <jim(a)meyering.net> wrote:
Without this patch, building without HAVE_AVAHI fails due to an
unguarded
use of mdns_name. Rather than adding yet another #ifdef..#endif (ugly
and less maintainable), I've chosen to un-ifdef the declarations and
a few uses. The more code that we compile and use unconditionally,
the fewer surprises we'll encounter down the road.
Mon Dec 3 14:24:20 CET 2007 Jim Meyering <meyering(a)redhat.com>
Avoid compile failure when HAVE_AVAHI is not defined.
* qemud/qemud.c (remoteReadConfigFile): Remove some of the
"#ifdef HAVE_AVAHI" guards around uses of mdns_name and mdns_adv.
FYI, after some off-list discussion, there are no more objections
to this patch, so I've committed it.
6230
days inactive
6230
days old
7 comments
4 participants
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering -
Richard W.M. Jones