On Mon, Sep 19, 2011 at 10:20:54AM -0500, Dan McGee wrote:
> On Mon, Sep 19, 2011 at 10:13 AM, Jonathan Neuschäfer
> <j.neuschae...@gmx.net> wrote:
> > Just some minor nitpicks here, I'm not an MPD core developer.
> >
> > On Mon, Sep 19, 2011 at 08:10:13AM -0500, Dan McGee wrote:
> >> +     if (fd >= 0) {
> >> +             if (socket_keepalive(fd))
> >> +                     g_warning("Could not set TCP keepalive option: %s",
> >> +                               g_strerror(errno));
> >
> > You use socket_keepalive's return value like a bool here, maybe it
> > should really return bool, as tag_ape_scan (src/ape.h, just a random
> > example) does.
> setsockopt() is documented to return only 0 or -1 (on windows, the
> #defined SOCKET_ERROR == -1), so it more or less is a boolean. There
> were no other bool-returning functions here so I didn't want to
> introduce a new precedent.

Makes sense.

> >> +#ifdef WIN32
> >> +     const char *optval = (const char *)&reuse;
> >
> > This workaround could use a comment (if it isn't obvious to people
> > who have used setsockopt on Windows before).
> I figured because this workaround matches the exact same one used a
> few lines above in socket_bind_listen(), it wouldn't be too magic. If
> you'd like a follow-on patch to document it in both places I can do
> that.

Well, I let Max decide :-).

        Jonathan

------------------------------------------------------------------------------
BlackBerry&reg; DevCon Americas, Oct. 18-20, San Francisco, CA
Learn about the latest advances in developing for the 
BlackBerry&reg; mobile platform with sessions, labs & more.
See new tools and technologies. Register for BlackBerry&reg; DevCon today!
http://p.sf.net/sfu/rim-devcon-copy1 
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team

Reply via email to