'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/]
