Re: [libvirt PATCH] wireshark: Don't include config.h

[Laine Stump <laine@xxxxxxxxxx>]

On 9/3/20 6:43 AM, Andrea Bolognani wrote:
> While both Debian and Fedora include the header in their development
> packages for Wireshark, that's not something that the upstream
> developers intended and arguably quite wrong, as config.h is obviously
> intended to only be used to drive the compilation of Wireshark itself.
> The Arch Linux package behaves like the upstream Wireshark package, and
> thus libvirt fails to build there.
>
> It seems that there are multiple bugs to be addressed:
>
>    * libvirt shouldn't include config.h;
>
>    * Debian and Fedora shouldn't be shipping config.h in their Wireshark
>      packages;
>
>    * Wireshark should not use config.h defines such as HAVE_PLUGINS in
>      its public headers, and define a public variant of them instead.
>
> This patch takes care of the first one.
>
> https://gitlab.com/libvirt/libvirt/-/issues/74
> Signed-off-by: Andrea Bolognani <abologna@xxxxxxxxxx>


Funny, just this morning I was grepping for " HAVE_" and " WITH_" in 
/usr/include, saw a couple of "config.h"s shoot past, and thought "Hmm. 
That doesn't seem right! Surely those packages didn't *really* intend to 
ship their config.h with their -dev package".


Reviewed-by: Laine Stump <laine@xxxxxxxxxx>


> ---
>   tools/wireshark/src/packet-libvirt.c |  1 -
>   tools/wireshark/src/plugin.c         | 13 +++++++------
>   2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
> index 9f3c7f650d..965f1f5482 100644
> --- a/tools/wireshark/src/packet-libvirt.c
> +++ b/tools/wireshark/src/packet-libvirt.c
> @@ -18,7 +18,6 @@
>    */
>   #include <config.h>
>   
> -#include <wireshark/config.h>
>   #include <wireshark/epan/proto.h>
>   #include <wireshark/epan/packet.h>
>   #include <wireshark/epan/dissectors/packet-tcp.h>
> diff --git a/tools/wireshark/src/plugin.c b/tools/wireshark/src/plugin.c
> index 504e4383a7..3c6fae9ef5 100644
> --- a/tools/wireshark/src/plugin.c
> +++ b/tools/wireshark/src/plugin.c
> @@ -14,11 +14,12 @@
>   
>   #include <gmodule.h>
>   
> -#include <wireshark/config.h>
> +#define HAVE_PLUGINS 1
>   #include <wireshark/epan/proto.h>
>   /* plugins are DLLs */
>   #define WS_BUILD_DLL
>   #include <wireshark/ws_symbol_export.h>
> +#include <wireshark/ws_version.h>
>   
>   #include "packet-libvirt.h"
>   
> @@ -26,9 +27,9 @@
>   #define PLUGIN_VERSION VERSION
>   
>   #define WIRESHARK_VERSION \
> -    ((VERSION_MAJOR * 1000 * 1000) + \
> -     (VERSION_MINOR * 1000) + \
> -     (VERSION_MICRO))
> +    ((WIRESHARK_VERSION_MAJOR * 1000 * 1000) + \
> +     (WIRESHARK_VERSION_MINOR * 1000) + \
> +     (WIRESHARK_VERSION_MICRO))
>   
>   #if WIRESHARK_VERSION < 2005000
>   
> @@ -69,8 +70,8 @@ void proto_register_libvirt(void);
>   void proto_reg_handoff_libvirt(void);
>   
>   WS_DLL_PUBLIC_DEF const gchar plugin_version[] = PLUGIN_VERSION;
> -WS_DLL_PUBLIC_DEF const int plugin_want_major = VERSION_MAJOR;
> -WS_DLL_PUBLIC_DEF const int plugin_want_minor = VERSION_MINOR;
> +WS_DLL_PUBLIC_DEF const int plugin_want_major = WIRESHARK_VERSION_MAJOR;
> +WS_DLL_PUBLIC_DEF const int plugin_want_minor = WIRESHARK_VERSION_MINOR;
>   
>   WS_DLL_PUBLIC void plugin_register(void);
>