On Wed, 11.02.09 11:12, Iain Hibbert (plunky at rya-online.net) wrote:
Patch looks mostly good. Except this:
> - if (connect(fd, af == AF_INET ? (struct sockaddr*) &sa4 : (struct sockaddr*) &sa6, (socklen_t) (af == AF_INET ? sizeof(sa4) : sizeof(sa6))) < 0) {
> + if (af == AF_INET && connect(fd, (struct sockaddr*) &sa4, sizeof(sa4)) < 0) {
> pa_log("connect() failed: %s", pa_cstrerror(errno));
> goto fail;
> +#ifdef HAVE_IPV6
> + } else if (connect(fd, (struct sockaddr*) &sa6, sizeof(sa6)) < 0) {
> + pa_log("connect() failed: %s", pa_cstrerror(errno));
> + goto fail;
> +#endif
> }
In the first if evaluates to FALSE then the second one will be
excuted -- resulting on connect() being called twice if IPv4 worked
fine.
>
> if ((sap_fd = socket(af, SOCK_DGRAM, 0)) < 0) {
> @@ -277,9 +286,14 @@
> goto fail;
> }
>
> - if (connect(sap_fd, af == AF_INET ? (struct sockaddr*) &sap_sa4 : (struct sockaddr*) &sap_sa6, (socklen_t) (af == AF_INET ? sizeof(sap_sa4) : sizeof(sap_sa6))) < 0) {
> + if (af == AF_INET && connect(sap_fd, (struct sockaddr*) &sap_sa4, sizeof(sap_sa4)) < 0) {
> + pa_log("connect() failed: %s", pa_cstrerror(errno));
> + goto fail;
> +#ifdef HAVE_IPV6
> + } else if (connect(sap_fd, (struct sockaddr*) &sap_sa6, sizeof(sap_sa6)) < 0) {
> pa_log("connect() failed: %s", pa_cstrerror(errno));
> goto fail;
> +#endif
> }
Same here.
> +#ifdef HAVE_IPV6
> pa_socket_server* pa_socket_server_new_ipv6(pa_mainloop_api *m, const uint8_t address[16], uint16_t port, const char *tcpwrap_service) {
> pa_socket_server *ss;
> int fd = -1;
> @@ -347,6 +348,7 @@
>
> return NULL;
> }
> +#endif
Please #ifdef out the functions in the header files as well.
Also, please make sure to follow our indentation rules:
http://pulseaudio.org/wiki/CodingStyle
If you fix these few issues I will merge the patch. If you hurry up I
will even include it in 0.9.15.
Thanks,
Lennart
--
Lennart Poettering Red Hat, Inc.
lennart [at] poettering [dot] net ICQ# 11060553
http://0pointer.net/lennart/ GnuPG 0x1A015CC4
