Bug 41538

The volume fixes look good to me. What is the socket-server.c change all about? To me it looks like you create two global variables that are not used anywhere...

Your question is about this hunk:

--- pulseaudio-0.9.22/src/pulsecore/socket-server.c-orig	2011-06-09 16:43:59.167250309 -0500
+++ pulseaudio-0.9.22/src/pulsecore/socket-server.c	2011-06-09 16:59:12.482110908 -0500
@@ -44,7 +44,12 @@
 #endif
 
 #ifdef HAVE_LIBWRAP
+#ifdef __sun
+#include <syslog.h>
+int allow_severity = LOG_INFO;
+int deny_severity = LOG_WARNING;
+#endif
 #include <tcpd.h>
 #endif

---

On Linux, the libwrap library includes the allow_severity and deny_severity symbols.  You can look at it and check.  On Solaris, these interfaces are referenced by the library but the symbols are not in the library.  The expectation is that on Solaris, the client program defines them.  Since Solaris handles this differently, the code is wrapped in "#ifdef __sun".

You will notice that other programs that use libwrap, such as GDM (see gdm/daemon/gdm-xdmcp-display-factory.c) make use of the same "#ifdef __sun" code.

I do not think this particular #ifdef clutters the code overmuch, and it does make the code buildable on Solaris, so it would be nice if it could go upstream.

(In reply to comment #2)
>  #ifdef HAVE_LIBWRAP
> +#ifdef __sun
> +#include <syslog.h>
> +int allow_severity = LOG_INFO;
> +int deny_severity = LOG_WARNING;
> +#endif
>  #include <tcpd.h>
>  #endif

Couldn't these be #defines, or is that not compatible with how tcpd.h uses the symbols?

The LOG_INFO and LOG_WARNING symbols are defined by pulse, isn't it? How is it that our definition complies with that of libwrap?

Furthermore a one line comment about why this is necessary on solaris could help to satisfy Tanu here.

Created attachment 52504 [details] [review]
updated patch

I've attached an updated patch that properly uses "#ifdef __sun" for the Solaris specific LIBWRAP code, and also adds a comment to explain this code, as requested.

On Solaris, LOG_INFO and LOG_WARNING are defrine din /usr/include/sys/syslog.h (included by /usr/include/syslog.h), which is why the #ifdef __sun code includes syslog.h.  These are not defined by Pulse.

Thanks -- the patches have been pushed and will be in the next (1.1) release that is due shortly.