[Libvir] [PATCH] Avoid compile failure when HAVE_AVAHI is not defined.
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@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
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
"Richard W.M. Jones" <rjones@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?
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 -=|
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
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 -=|
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@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@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Jim Meyering <jim@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@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.
participants (4)
-
Daniel P. Berrange -
Daniel Veillard -
Jim Meyering -
Richard W.M. Jones