Bug 41538 - Solaris compile fixes for PulseAudio 1.0
Summary: Solaris compile fixes for PulseAudio 1.0
Status: RESOLVED FIXED
Alias: None
Product: PulseAudio
Classification: Unclassified
Component: misc (show other bugs)
Version: unspecified
Hardware: Other Solaris
: medium normal
Assignee: pulseaudio-bugs
QA Contact: pulseaudio-bugs
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-10-06 14:20 UTC by Brian Cameron
Modified: 2011-10-20 02:37 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
patch fixing issue (1.39 KB, patch)
2011-10-06 14:20 UTC, Brian Cameron
Details | Splinter Review
updated patch (1.50 KB, patch)
2011-10-18 17:46 UTC, Brian Cameron
Details | Splinter Review

Description Brian Cameron 2011-10-06 14:20:59 UTC
Created attachment 52059 [details] [review]
patch fixing issue

The attached patch includes a few minor fixes that make it build properly on Solaris.  The fix to module-solaris.c seems to be an issue that the code was not updated to use real_volume instead of volume when the rest of the modules were.
Comment 1 Tanu Kaskinen 2011-10-07 15:28:46 UTC
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...
Comment 2 Brian Cameron 2011-10-10 08:40:58 UTC
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.
Comment 3 Maarten Bosmans 2011-10-16 05:58:44 UTC
(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.
Comment 4 Brian Cameron 2011-10-18 17:46:50 UTC
Created attachment 52504 [details] [review]
updated patch
Comment 5 Brian Cameron 2011-10-18 18:02:14 UTC
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.
Comment 6 Arun Raghavan 2011-10-20 02:37:05 UTC
Thanks -- the patches have been pushed and will be in the next (1.1) release that is due shortly.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.