[pulseaudio-discuss] Pull request for topic/osx

[Colin Guthrie]

'Twas brillig, and Daniel Mack at 10/12/09 02:49 did gyre and gimble:
> Colin, I'd take your offer and ask you to pull the tree I mentioned
> above - I will not touch it anymore until you merged :)

Excellent!

Got a couple quick comments (regardless of what I said on IRC!!)



There are a couple places where it appears a final #else.... #endif has
been removed... this means code will be compiled that isn't needed (the
previous matching #if/#elif block ends with a return) Some compilers
throw up warnings about unreachable code in this case. It's probably
best to ensure the #else #endif is restored (although, personally I
prefer to have a return right before the closing brace, I don't want to
mess with compiler warnings nor Lennart's current code style :))

Files affected:
 src/pulsecore/core-rtclock.c (two occurrences relating to
HAVE_CLOCK_GETTIME)


--- a/src/pulsecore/poll.c
+++ b/src/pulsecore/poll.c

+/* Mac OSX fails to implement poll() in a working way since 10.4. IOW, for
+ * several years. We need to enable a dirty workaround and emulate that
call
+ * with select(), just like for Windows. sic! */
+
+#if defined(HAVE_POLL_H) && !defined(OS_IS_DARWIN)
+
+int pa_poll (struct pollfd *fds, unsigned long int nfds, int timeout) {
+       return poll(fds, nfds, timeout);
+}
+
+#else

Would this be better as a:
 #define pa_poll poll
?? I'm not 100% sure here but I guess most compilers would optimise this
out anyway... this is more of a "hmmm" than a specific question I guess.
Happy to leave it as it is for now until someone can comment more
authoritatively than me on the matter :D

--- a/src/pulsecore/poll.h
+++ b/src/pulsecore/poll.h

-extern int poll (struct pollfd *__fds, nfds_t __nfds, int __timeout);
+extern int pa_poll (struct pollfd *fds, unsigned long nfds, int timeout);

Should this be marked as extern now? From what I can tell pa_poll is
fully implemented internally now (tho' my #define comment above may
change that!)


Perhaps this would be better:

--- a/src/pulsecore/poll.h
+++ b/src/pulsecore/poll.h

+#ifdef HAVE_POLL_H
+#include <poll.h>
+#define pa_poll poll
+#else
+
  blah
+ int pa_poll (struct pollfd *fds, unsigned long nfds, int timeout);
+#endif

Then miss out the whole extern and the pass-through function in poll.c?

I'm just really thinking out loud about what the cleanest and most
efficient structure here... comments welcome :)


Also, where does the ppoll stuff used in rtpoll.h/c and mainloop.c? Is
there any way we could wrap that up (doesn't look like it on first
glance). It would be nice if we could just use pa_poll() and not worry
about variations (with all the clever stuff done in poll.c/h).

Anyway, like I say just thinking out loud main. Let me know what you
think and whether any of the above makes any sense!

This is only a small part of your overall changes but is the most
"corey" bit - the rest of the stuff does indeed look great.

Thanks loads and hope my comments make sense! :)

Col
-- 

Colin Guthrie
gmane(at)colin.guthr.ie
http://colin.guthr.ie/

Day Job:
  Tribalogic Limited [http://www.tribalogic.net/]
Open Source:
  Mandriva Linux Contributor [http://www.mandriva.com/]
  PulseAudio Hacker [http://www.pulseaudio.org/]
  Trac Hacker [http://trac.edgewall.org/]