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.
>>
>> return fd;
>> }
>> +
>> +int
>> +socket_keepalive(int fd)
>> +{
>> + const int reuse = 1;
>> +
>> +#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.
>> +int
>> +socket_keepalive(int fd);
>
> A doxygen comment would probably be quite appropriate here.
Quite true, I can do this.
>
> Thanks,
> Jonathan Neuschäfer
------------------------------------------------------------------------------
BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA
Learn about the latest advances in developing for the
BlackBerry® mobile platform with sessions, labs & more.
See new tools and technologies. Register for BlackBerry® 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
