Hello Gunnar,

I saw you uploaded your win32 patch to the git repository
http://git.musicpd.org/cgit/rodion/mpd.git/, but I cannot merge this
patch.  You did not obey the most basic rules I explained to you on
IRC.

> commit 6c1d9df29ad22c6ef13528718a3d91fcff34819f
> Author: Gunnar Roth <gun...@garmedia.(none)>

That email address is invalid.

> Date:   Sun Jan 11 21:43:40 2009 +0100
> 
>     changes to make mpd work on win32 using gcc.
>     
>     this patches misses all changes for visal c compiler, this will be on a 
> different patch.
>     it just should compile with mingw but not tested. i have tested, that it 
> compiles on linux.
>     so the changes should not break linux.

You are mixing a huge amounts of different changes in one single
patch.  Don't do this, please.  Your patch description doesn't
describe the patch at all, it's generic blah.

And yes, you *are* breaking the Linux build:

 event_pipe.c: In function 'main_notify_event':
 event_pipe.c:65: error: comparison of unsigned expression < 0 is
 always false

> diff --git a/src/client.c b/src/client.c
> old mode 100644
> new mode 100755
> index e059f64..ac10811
> --- a/src/client.c
> +++ b/src/client.c
> @@ -34,9 +34,8 @@
>  #include <stdio.h>
>  #include <errno.h>
>  
> -#ifdef WIN32
> -#include <ws2tcpip.h>
> -#include <winsock.h>
> +#ifdef _WIN32
> +#include <WinSock2.h>

No uppercase letters in file names.

>  #else
>  #include <sys/socket.h>
>  #include <netinet/in.h>
> @@ -78,7 +77,7 @@ struct client {
>       int fd; /* file descriptor; -1 if expired */
>       GIOChannel *channel;
>       guint source_id;
> -
> +     

>       unsigned permission;
>  

What's the goal of this hunk?

>       /** the uid of the client process, or -1 if unknown */
> @@ -114,6 +113,9 @@ static void client_write_deferred(struct client *client);
>  
>  static void client_write_output(struct client *client);
>  
> +static void client_write_direct(struct client *client,
> +                                                             const char 
> *data, size_t length);
> +
>  bool client_is_expired(const struct client *client)
>  {
>       return client->fd < 0;
> @@ -166,7 +168,11 @@ static inline void client_set_expired(struct client 
> *client)
>       }
>  
>       if (client->fd >= 0) {
> +#ifdef _WIN32
> +             closesocket(client->fd);
> +#else
>               close(client->fd);
> +#endif
>               client->fd = -1;
>       }
>  }

Instead of doing an #ifdef whenever a socket is closed, add a wrapper
for that.  Too many #ifdefs make the code unreadable.


> @@ -186,8 +192,12 @@ static void client_init(struct client *client, int fd)
>       client->bufferPos = 0;
>       client->fd = fd;
>  
> +#ifdef _WIN32
> +     client->channel = g_io_channel_win32_new_socket(client->fd);
> +#else
>       client->channel = g_io_channel_unix_new(client->fd);
> -     client->source_id = g_io_add_watch(client->channel, G_IO_IN,
> +#endif
> +     client->source_id = g_io_add_watch(client->channel, G_IO_IN|G_IO_NVAL | 
> G_IO_HUP | G_IO_ERR,
>                                          client_in_event, client);
>  
>       client->lastTime = time(NULL);
> @@ -199,7 +209,7 @@ static void client_init(struct client *client, int fd)
>  
>       client->permission = getDefaultPermissions();
>  
> -     write(fd, GREETING, sizeof(GREETING) - 1);
> +     client_write_direct(client, GREETING, sizeof(GREETING) - 1);
>  }
>  
>  static void free_cmd_list(GSList *list)
> @@ -293,7 +303,11 @@ void client_new(int fd, const struct sockaddr *addr, int 
> uid)
>  
>       if (num_clients >= client_max_connections) {
>               g_warning("Max Connections Reached!");
> +#ifdef _WIN32
> +             closesocket(fd);
> +#else
>               close(fd);
> +#endif

See above.

>               return;
>       }
>  
> @@ -305,7 +319,7 @@ void client_new(int fd, const struct sockaddr *addr, int 
> uid)
>       client->uid = uid;
>       g_log(G_LOG_DOMAIN, LOG_LEVEL_SECURE,
>             "client %i: opened from %s\n", client->num,
> -           sockaddr_to_tmp_string(addr));
> +            sockaddr_to_tmp_string(addr));
>  }

???

>  
>  static int client_process_line(struct client *client, char *line)
> @@ -329,7 +343,7 @@ static int client_process_line(struct client *client, 
> char *line)
>               /* during idle mode, clients must not send anything
>                  except "noidle" */
>               g_warning("client %i: command \"%s\" during idle",
> -                       client->num, line);
> +                   client->num, line);
>               return COMMAND_RETURN_CLOSE;
>       }

???

>  
> @@ -367,8 +381,8 @@ static int client_process_line(struct client *client, 
> char *line)
>                           client_max_command_list_size) {
>                               g_warning("client %i: command list size (%lu) "
>                                         "is larger than the max (%lu)",
> -                                       client->num,
> -                                       (unsigned long)client->cmd_list_size,
> +                                   client->num,
> +                                   (unsigned long)client->cmd_list_size,
>                                         (unsigned 
> long)client_max_command_list_size);
>                               return COMMAND_RETURN_CLOSE;
>                       } else

???

> @@ -383,10 +397,10 @@ static int client_process_line(struct client *client, 
> char *line)
>                       ret = 1;
>               } else {
>                       g_debug("client %i: process command \"%s\"",
> -                             client->num, line);
> +                           client->num, line);
>                       ret = command_process(client, line);
>                       g_debug("client %i: command returned %i",
> -                             client->num, ret);
> +                           client->num, ret);
>  
>                       if (ret == COMMAND_RETURN_CLOSE ||
>                           client_is_expired(client))

???

> @@ -440,7 +454,7 @@ static int client_input_received(struct client *client, 
> size_t bytesRead)
>       if (client->bufferLength == sizeof(client->buffer)) {
>               if (client->bufferPos == 0) {
>                       g_warning("client %i: buffer overflow",
> -                               client->num);
> +                           client->num);
>                       return COMMAND_RETURN_CLOSE;
>               }
>               assert(client->bufferLength >= client->bufferPos

???

> @@ -462,13 +476,22 @@ static int client_read(struct client *client)
>       assert(client->bufferPos <= client->bufferLength);
>       assert(client->bufferLength < sizeof(client->buffer));
>  
> -     bytesRead = read(client->fd,
> +#ifdef _WIN32
> +     bytesRead = recv(client->fd,
>                        client->buffer + client->bufferLength,
> -                      sizeof(client->buffer) - client->bufferLength);
> -
> +                      sizeof(client->buffer) - client->bufferLength,0);
> +#else
> +     bytesRead = read(client->fd,
> +             client->buffer + client->bufferLength,
> +             sizeof(client->buffer) - client->bufferLength);
> +#endif

Why yet another #ifdef hack, when you could just replace read() with
recv()?  On POSIX compliant systems, both is allowed, while Windows
only allows recv().

>       if (bytesRead > 0)
>               return client_input_received(client, bytesRead);
> +#ifdef _WIN32
> +     else if (bytesRead < 0 && WSAGetLastError() == WSAEINTR)
> +#else
>       else if (bytesRead < 0 && errno == EINTR)
> +#endif
>               /* try again later, after select() */
>               return 0;
>       else
> @@ -483,13 +506,18 @@ client_out_event(G_GNUC_UNUSED GIOChannel *source,
>  
>  static gboolean
>  client_in_event(G_GNUC_UNUSED GIOChannel *source,
> -             G_GNUC_UNUSED GIOCondition condition,
> +             GIOCondition condition,
>               gpointer data)
>  {
>       struct client *client = data;
>       int ret;
>  
>       assert(!client_is_expired(client));
> +     if (condition != G_IO_IN) {
> +             client_set_expired(client);
> +             return false;
> +     }
> +
>  
>       client->lastTime = time(NULL);
>  
> @@ -542,7 +570,7 @@ client_out_event(G_GNUC_UNUSED GIOChannel *source,
>       if (g_queue_is_empty(client->deferred_send)) {
>               /* done sending deferred buffers exist: schedule
>                  read */
> -             client->source_id = g_io_add_watch(client->channel, G_IO_IN,
> +             client->source_id = g_io_add_watch(client->channel, 
> G_IO_IN|G_IO_NVAL | G_IO_HUP | G_IO_ERR,
>                                                  client_in_event, client);
>               return false;
>       }

Care to explain this one?  On Unix, it's enough to listen for G_IO_IN,
since the socket becomes readable when the peer closes the connection.

> @@ -630,17 +658,17 @@ client_check_expired_callback(gpointer data, 
> G_GNUC_UNUSED gpointer user_data)
>  {
>       struct client *client = data;
>  
> -     if (client_is_expired(client)) {
> +             if (client_is_expired(client)) {

???

>               g_debug("client %i: expired", client->num);
> -             client_close(client);
> -     } else if (!client->idle_waiting && /* idle clients
> -                                            never expire */
> -                time(NULL) - client->lastTime >
> -                client_timeout) {
> +                     client_close(client);
> +             } else if (!client->idle_waiting && /* idle clients
> +                                                    never expire */
> +                        time(NULL) - client->lastTime >
> +                        client_timeout) {
>               g_debug("client %i: timeout", client->num);
> -             client_close(client);
> +                     client_close(client);
> +             }
>       }
> -}

??????

>  static void
>  client_manager_expire(void)
> @@ -659,7 +687,11 @@ static void client_write_deferred(struct client *client)
>               assert(buf->size > 0);
>               assert(buf->size <= client->deferred_bytes);
>  
> +#ifdef _WIN32        
> +             ret = send(client->fd, buf->data, buf->size,0);
> +#else
>               ret = write(client->fd, buf->data, buf->size);
> +#endif
>               if (ret < 0)

Yet another ugly and superfluous #ifdef.

>                       break;
>               else if ((size_t)ret < buf->size) {
> @@ -682,12 +714,16 @@ static void client_write_deferred(struct client *client)
>  
>       if (g_queue_is_empty(client->deferred_send)) {
>               g_debug("client %i: buffer empty %lu", client->num,
> -                     (unsigned long)client->deferred_bytes);
> +                   (unsigned long)client->deferred_bytes);

???

>               assert(client->deferred_bytes == 0);
> +#ifdef _WIN32
> +     } else if (ret < 0 && WSAGetLastError() != WSAEWOULDBLOCK && 
> WSAGetLastError() != WSAEINTR) {
> +#else
>       } else if (ret < 0 && errno != EAGAIN && errno != EINTR) {
> +#endif
>               /* cause client to close */
>               g_debug("client %i: problems flushing buffer",
> -                     client->num);
> +                   client->num);

???

>               client_set_expired(client);
>       }
>  }
> @@ -705,9 +741,9 @@ static void client_defer_output(struct client *client,
>       if (client->deferred_bytes > client_max_output_buffer_size) {
>               g_warning("client %i: output buffer size (%lu) is "
>                         "larger than the max (%lu)",
> -                       client->num,
> -                       (unsigned long)client->deferred_bytes,
> -                       (unsigned long)client_max_output_buffer_size);
> +                   client->num,
> +                   (unsigned long)client->deferred_bytes,
> +                   (unsigned long)client_max_output_buffer_size);
>               /* cause client to close */
>               client_set_expired(client);
>               return;

????


> @@ -728,8 +764,13 @@ static void client_write_direct(struct client *client,
>       assert(length > 0);
>       assert(g_queue_is_empty(client->deferred_send));
>  
> +#ifdef _WIN32
> +     if ((ret = send(client->fd, data, length,0)) < 0) {
> +             if (WSAGetLastError() == WSAEWOULDBLOCK || WSAGetLastError() == 
> WSAEINTR) {
> +#else
>       if ((ret = write(client->fd, data, length)) < 0) {
>               if (errno == EAGAIN || errno == EINTR) {
> +#endif
>                       client_defer_output(client, data, length);
>               } else {
>                       g_debug("client %i: problems writing", client->num);

See above.

> diff --git a/src/cmdline.c b/src/cmdline.c
> old mode 100644
> new mode 100755
> index 48c1e87..ef7dd2c
> --- a/src/cmdline.c
> +++ b/src/cmdline.c
> @@ -32,7 +32,11 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  
> +#ifdef _WIN32
> +#define SYSTEM_CONFIG_FILE_LOCATION  "c:/.mpd/mpd.conf"
> +#else
>  #define SYSTEM_CONFIG_FILE_LOCATION  "/etc/mpd.conf"
> +#endif
>  #define USER_CONFIG_FILE_LOCATION    ".mpdconf"

Is that actually a good default location for win32?  This looks like
it violates Microsoft's policies.

>  G_GNUC_NORETURN
> diff --git a/src/command.c b/src/command.c
> old mode 100644
> new mode 100755
> index 1fac4fd..341a1e8
> --- a/src/command.c
> +++ b/src/command.c
> @@ -525,12 +525,8 @@ handle_add(struct client *client, G_GNUC_UNUSED int 
> argc, char *argv[])
>       enum playlist_result result;
>  
>       if (strncmp(uri, "file:///", 8) == 0) {
> -#ifdef WIN32
> -             result = PLAYLIST_RESULT_DENIED;
> -#else
>               result = playlist_append_file(uri + 7, client_get_uid(client),
>                                             NULL);
> -#endif
>               return print_playlist_result(client, result);
>       }
>  

I disabled this code on Win32 for a good reason, which can be read in
the according patch.  Please explain this hunk.

> @@ -561,14 +557,10 @@ handle_addid(struct client *client, int argc, char 
> *argv[])
>       unsigned added_id;
>       enum playlist_result result;
>  
> -     if (strncmp(uri, "file:///", 8) == 0) {
> -#ifdef WIN32
> -             result = PLAYLIST_RESULT_DENIED;
> -#else
> +     if (strncmp(uri, "file:///", 8) == 0){
>               result = playlist_append_file(uri + 7,
>                                             client_get_uid(client),
>                                             &added_id);
> -#endif
>       } else {
>               if (uri_has_scheme(uri) && !uri_supported_scheme(uri)) {
>                       command_error(client, ACK_ERROR_NO_EXIST,

See above.

> diff --git a/src/daemon.c b/src/daemon.c
> old mode 100644
> new mode 100755
> index 913f06f..e194496
> --- a/src/daemon.c
> +++ b/src/daemon.c
> @@ -31,6 +31,7 @@
>  void
>  daemonize_close_stdin(void)
>  {
> +#ifndef WIN32
>       int fd = open("/dev/null", O_RDONLY);
>  
>       if (fd < 0)
> @@ -39,6 +40,7 @@ daemonize_close_stdin(void)
>               dup2(fd, STDIN_FILENO);
>               close(fd);
>       }
> +#endif
>  }
>  
>  void
> diff --git a/src/database.h b/src/database.h
> old mode 100644
> new mode 100755
> index f0f2251..21aa228
> --- a/src/database.h
> +++ b/src/database.h
> @@ -20,7 +20,11 @@
>  #ifndef MPD_DATABASE_H
>  #define MPD_DATABASE_H
>  
> +#ifdef _WIN32
> +#include <time.h>
> +#else
>  #include <sys/time.h>
> +#endif
>  #include <stdbool.h>
>  
>  struct directory;
> diff --git a/src/event_pipe.c b/src/event_pipe.c
> old mode 100644
> new mode 100755
> index 2bfdf20..0f672af
> --- a/src/event_pipe.c
> +++ b/src/event_pipe.c
> @@ -53,15 +53,16 @@ event_pipe_invoke(enum pipe_event event)
>  }
>  
>  static gboolean
> -main_notify_event(G_GNUC_UNUSED GIOChannel *source,
> +main_notify_event(GIOChannel *source,
>                 G_GNUC_UNUSED GIOCondition condition,
>                 G_GNUC_UNUSED gpointer data)
>  {
>       char buffer[256];
> -     ssize_t r = read(event_pipe[0], buffer, sizeof(buffer));
> +     gsize bytes_read = 0;
> +     GIOStatus status = 
> g_io_channel_read_chars(source,buffer,1,&bytes_read,NULL);
>       bool events[PIPE_EVENT_MAX];
>  
> -     if (r < 0 && errno != EAGAIN && errno != EINTR)
> +     if (bytes_read < 0 && status != G_IO_STATUS_NORMAL && status != 
> G_IO_STATUS_AGAIN)
>               g_error("error reading from pipe: %s", strerror(errno));
>  
>       g_mutex_lock(event_pipe_mutex);

This hunk is wrong and breaks the Linux build, and should also break
the Windows build.

> @@ -76,10 +77,9 @@ main_notify_event(G_GNUC_UNUSED GIOChannel *source,
>  
>       return true;
>  }
> -
> +static GIOChannel *channel;
>  void event_pipe_init(void)
>  {
> -     GIOChannel *channel;
>       int ret;
>  
>  #ifdef WIN32
> @@ -89,14 +89,16 @@ void event_pipe_init(void)
>  #endif
>       if (ret < 0)
>               g_error("Couldn't open pipe: %s", strerror(errno));
> +#ifdef _WIN32
> +     channel = g_io_channel_win32_new_fd(event_pipe[0]);
> +#else        
>       if (set_nonblocking(event_pipe[1]) < 0)
>               g_error("Couldn't set non-blocking I/O: %s", strerror(errno));
> -
>       channel = g_io_channel_unix_new(event_pipe[0]);
> +#endif
>       event_pipe_source_id = g_io_add_watch(channel, G_IO_IN,
>                                             main_notify_event, NULL);
> -     g_io_channel_unref(channel);
> -
> +     
>       event_pipe_mutex = g_mutex_new();
>  }
>  
> @@ -105,8 +107,11 @@ void event_pipe_deinit(void)
>       g_mutex_free(event_pipe_mutex);
>  
>       g_source_remove(event_pipe_source_id);
> -
> -     close(event_pipe[0]);
> +     
> +     g_io_channel_shutdown(channel,false,NULL);
> +     g_io_channel_unref(channel);
> +     channel = NULL;
> +     
>       close(event_pipe[1]);
>  }

The whole event_pipe.c patch requires explanation.

> diff --git a/src/input_curl.c b/src/input_curl.c
> old mode 100644
> new mode 100755
> index f863865..588bde5
> --- a/src/input_curl.c
> +++ b/src/input_curl.c
> @@ -23,7 +23,9 @@
>  #include "icy_metadata.h"
>  
>  #include <assert.h>
> +#ifndef _WIN32
>  #include <sys/select.h>
> +#endif
>  #include <string.h>
>  #include <errno.h>
>  
> diff --git a/src/input_file.c b/src/input_file.c
> old mode 100644
> new mode 100755
> index 5eee810..ee2c7f9
> --- a/src/input_file.c
> +++ b/src/input_file.c
> @@ -30,11 +30,13 @@ input_file_open(struct input_stream *is, const char 
> *filename)
>  {
>       int fd, ret;
>       struct stat st;
> -
> +#ifndef _WIN32
>       if (filename[0] != '/')
>               return false;
> -
>       fd = open(filename, O_RDONLY);
> +#else
> +     fd = open(filename, O_RDONLY|O_BINARY);
> +#endif
>       if (fd < 0) {
>               is->error = errno;
>               return false;
> diff --git a/src/listen.c b/src/listen.c
> old mode 100644
> new mode 100755
> index d0dfa3c..5bc1328
> --- a/src/listen.c
> +++ b/src/listen.c
> @@ -51,7 +51,7 @@
>  #define BINDERROR() do { \
>       g_error("unable to bind port %u: %s; " \
>               "maybe MPD is still running?", \
> -             port, strerror(errno)); \
> +           port, strerror(errno)); \
>  } while (0);

???

>  
>  struct listen_socket {
> @@ -88,7 +88,12 @@ static int establishListen(int pf, const struct sockaddr 
> *addrp,
>       }
>  
>       if (bind(sock, addrp, addrlen) < 0) {
> +#ifdef _WIN32
> +             closesocket(sock);
> +#else
>               close(sock);
> +#endif               
> +
>               return -1;
>       }
>  

See above.

> @@ -101,8 +106,11 @@ static int establishListen(int pf, const struct sockaddr 
> *addrp,
>  
>       ls = g_new(struct listen_socket, 1);
>       ls->fd = sock;
> -
> +#ifdef _WIN32
> +     channel = g_io_channel_win32_new_socket(sock);
> +#else
>       channel = g_io_channel_unix_new(sock);
> +#endif
>       ls->source_id = g_io_add_watch(channel, G_IO_IN,
>                                      listen_in_event, GINT_TO_POINTER(sock));
>       g_io_channel_unref(channel);
> @@ -120,7 +128,11 @@ static bool ipv6Supported(void)
>       s = socket(AF_INET6, SOCK_STREAM, 0);
>       if (s == -1)
>               return false;
> -     close(s);
> +#ifdef _WIN32
> +             closesocket(s);
> +#else
> +             close(s);
> +#endif               
>       return true;
>  #else
>       return false;

Same, see above.

> @@ -191,7 +203,7 @@ parseListenConfigParam(G_GNUC_UNUSED unsigned int port, 
> ConfigParam * param)
>  
>               if (establishListen(PF_UNIX, addrp, addrlen) < 0)
>                       g_error("unable to bind to %s: %s",
> -                             param->value, strerror(errno));
> +                           param->value, strerror(errno));
>  
>               /* allow everybody to connect */
>               chmod(param->value, 0666);

???

> @@ -199,7 +211,6 @@ parseListenConfigParam(G_GNUC_UNUSED unsigned int port, 
> ConfigParam * param)
>  #endif /* HAVE_UN */
>       } else {
>  #ifdef HAVE_TCP
> -#ifndef WIN32
>               struct addrinfo hints, *ai, *i;
>               char service[20];
>               int ret;
> @@ -220,7 +231,7 @@ parseListenConfigParam(G_GNUC_UNUSED unsigned int port, 
> ConfigParam * param)
>               ret = getaddrinfo(param->value, service, &hints, &ai);
>               if (ret != 0)
>                       g_error("can't lookup host \"%s\" at line %i: %s",
> -                             param->value, param->line, gai_strerror(ret));
> +                           param->value, param->line, gai_strerror(ret));
>  
>               for (i = ai; i != NULL; i = i->ai_next)
>                       if (establishListen(i->ai_family, i->ai_addr,

???

> @@ -228,23 +239,6 @@ parseListenConfigParam(G_GNUC_UNUSED unsigned int port, 
> ConfigParam * param)
>                               BINDERROR();
>  
>               freeaddrinfo(ai);
> -#else /* WIN32 */
> -             const struct hostent *he;
> -
> -             g_debug("binding to address for %s", param->value);
> -
> -             he = gethostbyname(param->value);
> -             if (he == NULL)
> -                     g_error("can't lookup host \"%s\" at line %i",
> -                             param->value, param->line);
> -
> -             if (he->h_addrtype != AF_INET)
> -                     g_error("IPv4 address expected for host \"%s\" at line 
> %i",
> -                             param->value, param->line);
> -
> -             if (establishListen(AF_INET, he->h_addr, he->h_length) < 0)
> -                     BINDERROR();
> -#endif /* !WIN32 */

Please explain.

>  #else /* HAVE_TCP */
>               g_error("TCP support is disabled");
>  #endif /* HAVE_TCP */
> @@ -264,7 +258,7 @@ void listenOnPort(void)
>                       g_error("%s \"%s\" specified at line %i is not a "
>                               "positive integer",
>                               CONF_PORT,
> -                             portParam->value, portParam->line);
> +                           portParam->value, portParam->line);
>               }
>       }
>  

???

> @@ -284,7 +278,11 @@ void closeAllListenSockets(void)
>               listen_sockets = ls->next;
>  
>               g_source_remove(ls->source_id);
> +#ifdef _WIN32
> +             closesocket(ls->fd);
> +#else
>               close(ls->fd);
> +#endif               
>               g_free(ls);
>       }
>  }

See above.

> diff --git a/src/main.c b/src/main.c
> old mode 100644
> new mode 100755
> index 03e8136..7d128a9
> --- a/src/main.c
> +++ b/src/main.c
> @@ -70,6 +70,8 @@
>  #ifndef WIN32
>  #include <pwd.h>
>  #include <grp.h>
> +#else
> +#include <windows.h>
>  #endif
>  
>  #ifdef HAVE_LOCALE_H
> @@ -96,7 +98,7 @@ static void changeToUser(void)
>  
>               if (setgid(userpwd->pw_gid) == -1) {
>                       g_error("cannot setgid for user \"%s\" at line %i: %s",
> -                             param->value, param->line, strerror(errno));
> +                           param->value, param->line, strerror(errno));
>               }
>  #ifdef _BSD_SOURCE
>               /* init suplementary groups 

???

> @@ -105,7 +107,7 @@ static void changeToUser(void)
>               if (initgroups(param->value, userpwd->pw_gid) == -1) {
>                       g_warning("cannot init supplementary groups "
>                                 "of user \"%s\" at line %i: %s",
> -                               param->value, param->line, strerror(errno));
> +                             param->value, param->line, strerror(errno));
>               }
>  #endif
>  

???

> @@ -113,7 +115,7 @@ static void changeToUser(void)
>               if (setuid(userpwd->pw_uid) == -1) {
>                       g_error("cannot change to uid of user "
>                               "\"%s\" at line %i: %s",
> -                             param->value, param->line, strerror(errno));
> +                           param->value, param->line, strerror(errno));
>               }
>  
>               /* this is needed by libs such as arts */

???

> @@ -126,7 +128,7 @@ static void changeToUser(void)
>  
>  static void openDB(Options * options, char *argv0)
>  {
> -     db_init();
> +             db_init();
>  
>       if (options->createDB > 0 || !db_load()) {
>               unsigned job;

???

> @@ -174,11 +176,11 @@ static void killFromPidFile(void)
>       fp = fopen(pidFileParam->value, "r");
>       if (!fp) {
>               g_error("unable to open %s \"%s\": %s",
> -                     CONF_PID_FILE, pidFileParam->value, strerror(errno));
> +                   CONF_PID_FILE, pidFileParam->value, strerror(errno));
>       }
>       if (fscanf(fp, "%i", &pid) != 1) {
>               g_error("unable to read the pid from file \"%s\"",
> -                     pidFileParam->value);
> +                   pidFileParam->value);
>       }
>       fclose(fp);
>  

???

> @@ -219,6 +221,14 @@ int main(int argc, char *argv[])
>       clock_t start;
>       GTimer *save_state_timer;
>       guint save_state_source_id;
> +#ifdef _WIN32
> +     WORD wVersionRequested;
> +     WSADATA wsaData;
> +
> +     wVersionRequested = MAKEWORD(2, 0);
> +     if (WSAStartup(wVersionRequested, &wsaData) != 0)
> +             return EXIT_FAILURE;
> +#endif
>  
>       daemonize_close_stdin();
>  
> @@ -245,7 +255,7 @@ int main(int argc, char *argv[])
>       tag_lib_init();
>       log_init(options.verbose, options.stdOutput);
>  
> -     listenOnPort();
> +             listenOnPort();
>  
>       changeToUser();
>  

???

> @@ -320,7 +330,7 @@ int main(int argc, char *argv[])
>       start = clock();
>       db_finish();
>       g_debug("db_finish took %f seconds",
> -             ((float)(clock()-start))/CLOCKS_PER_SEC);
> +           ((float)(clock()-start))/CLOCKS_PER_SEC);
>  
>       notify_deinit(&main_notify);
>       event_pipe_deinit();

???

> @@ -350,5 +360,8 @@ int main(int argc, char *argv[])
>       idle_deinit();
>  
>       close_log_files();
> +#ifdef _WIN32
> +     WSACleanup();
> +#endif
>       return EXIT_SUCCESS;
>  }
> diff --git a/src/playlist.c b/src/playlist.c
> old mode 100644
> new mode 100755
> index 8cffefc..7fe3c1c
> --- a/src/playlist.c
> +++ b/src/playlist.c
> @@ -151,12 +151,12 @@ void initPlaylist(void)
>       playlist.songMod = g_malloc(sizeof(playlist.songMod[0]) *
>                                   playlist_max_length);
>       playlist.order = g_malloc(sizeof(playlist.order[0]) *
> -                               playlist_max_length);
> +                              playlist_max_length);
>       playlist.idToPosition = g_malloc(sizeof(playlist.idToPosition[0]) *
>                                        playlist_max_length *
> -                                      PLAYLIST_HASH_MULT);
> +                                    PLAYLIST_HASH_MULT);
>       playlist.positionToId = g_malloc(sizeof(playlist.positionToId[0]) *
> -                                      playlist_max_length);
> +                                     playlist_max_length);
>  
>       memset(playlist.songs, 0, sizeof(char *) * playlist_max_length);
>  

???

> @@ -527,14 +527,13 @@ static void clearPlayerQueue(void)
>       pc_cancel();
>  }
>  
> -#ifndef WIN32
>  enum playlist_result
>  playlist_append_file(const char *path, int uid, unsigned *added_id)
>  {
>       int ret;
>       struct stat st;
>       struct song *song;
> -
> +#ifndef _WIN32 /* TODO: do user checks for WIN32 */
>       if (uid <= 0)
>               /* unauthenticated client */
>               return PLAYLIST_RESULT_DENIED;
> @@ -546,14 +545,13 @@ playlist_append_file(const char *path, int uid, 
> unsigned *added_id)
>       if (st.st_uid != (uid_t)uid && (st.st_mode & 0444) != 0444)
>               /* client is not owner */
>               return PLAYLIST_RESULT_DENIED;
> -
> +#endif
>       song = song_file_load(path, NULL);
>       if (song == NULL)
>               return PLAYLIST_RESULT_NO_SUCH_SONG;
>  
>       return addSongToPlaylist(song, added_id);
>  }
> -#endif
>  
>  static struct song *
>  song_by_url(const char *url)

See above - needs explanation why this fixes the win32 build.

> diff --git a/src/playlist.h b/src/playlist.h
> old mode 100644
> new mode 100755
> index 4af4aa8..af6a387
> --- a/src/playlist.h
> +++ b/src/playlist.h
> @@ -75,14 +75,13 @@ void savePlaylistState(FILE *);
>  
>  void clearPlaylist(void);
>  
> -#ifndef WIN32
> +
>  /**
>   * Appends a local file (outside the music database) to the playlist,
>   * but only if the file's owner is equal to the specified uid.
>   */
>  enum playlist_result
>  playlist_append_file(const char *path, int uid, unsigned *added_id);
> -#endif
>  
>  enum playlist_result addToPlaylist(const char *file, unsigned *added_id);
>  

See above.

> diff --git a/src/song.h b/src/song.h
> old mode 100644
> new mode 100755
> index a4c72cf..e717c07
> --- a/src/song.h
> +++ b/src/song.h
> @@ -21,7 +21,7 @@
>  
>  #include <stddef.h>
>  #include <stdbool.h>
> -#include <sys/time.h>
> +#include <time.h>
>  
>  #define SONG_BEGIN   "songList begin"
>  #define SONG_END     "songList end"
> diff --git a/src/tag.c b/src/tag.c
> old mode 100644
> new mode 100755
> index e7ce51c..d2f02cb
> --- a/src/tag.c
> +++ b/src/tag.c
> @@ -152,8 +152,11 @@ struct tag *tag_ape_load(const char *file)
>               TAG_ITEM_TRACK,
>               TAG_ITEM_DATE,
>       };
> -
> +#ifdef _Win32
> +     fp = fopen(file, "rb");
> +#else
>       fp = fopen(file, "r");
> +#endif
>       if (!fp)
>               return NULL;
>  

The "b" flag is portable, and the #ifdef can be removed.

> diff --git a/src/tag_id3.c b/src/tag_id3.c
> old mode 100644
> new mode 100755
> index b98d8af..96dd509
> --- a/src/tag_id3.c
> +++ b/src/tag_id3.c
> @@ -342,7 +342,11 @@ struct tag *tag_id3_load(const char *file)
>       struct id3_tag *tag;
>       FILE *stream;
>  
> +#ifdef _Win32
> +     stream = fopen(file, "rb");
> +#else
>       stream = fopen(file, "r");
> +#endif       
>       if (!stream) {
>               g_debug("tag_id3_load: Failed to open file: '%s', %s",
>                       file, strerror(errno));

See above.

> diff --git a/src/timer.c b/src/timer.c
> old mode 100644
> new mode 100755
> index 3cb497a..f231e31
> --- a/src/timer.c
> +++ b/src/timer.c
> @@ -24,7 +24,12 @@
>  
>  #include <assert.h>
>  #include <limits.h>
> +#ifdef _WIN32
> +#include <WinSock2.h> // for timeval
> +#include <time.h>
> +#else
>  #include <sys/time.h>
> +#endif
>  #include <stddef.h>
>  
>  static uint64_t now(void)

No uppercase file names, see above.

> diff --git a/src/update.c b/src/update.c
> old mode 100644
> new mode 100755
> index f272476..242a26e
> --- a/src/update.c
> +++ b/src/update.c
> @@ -273,6 +273,7 @@ statDirectory(struct directory *dir)
>  static int
>  inodeFoundInParent(struct directory *parent, ino_t inode, dev_t device)
>  {
> +#ifndef _WIN32
>       while (parent) {
>               if (!parent->stat && statDirectory(parent) < 0)
>                       return -1;
> @@ -282,7 +283,7 @@ inodeFoundInParent(struct directory *parent, ino_t inode, 
> dev_t device)
>               }
>               parent = parent->parent;
>       }
> -
> +#endif
>       return 0;
>  }
>  
> diff --git a/src/utils.c b/src/utils.c
> old mode 100644
> new mode 100755
> index ca0301e..542ce20
> --- a/src/utils.c
> +++ b/src/utils.c
> @@ -28,13 +28,16 @@
>  #include <fcntl.h>
>  #include <errno.h>
>  
> -#ifndef WIN32
> +#ifdef _WIN32
> +#include <WinSock2.h> // for timeval
> +#else

See above.

>  #include <pwd.h>
>  #endif
> -
>  #ifdef HAVE_IPV6
> +#ifndef _WIN32
>  #include <sys/socket.h>
>  #endif
> +#endif
>  
>  #ifdef WIN32
>  #include <windows.h>
> @@ -56,7 +59,14 @@ void my_usleep(long usec)
>  
>  char *parsePath(char *path)
>  {
> -#ifndef WIN32
> +
> +#ifdef _WIN32
> +     if (path[1] != ':' && path[0] != '/') {
> +             g_warning("\"%s\" is not an absolute path", path);
> +             return NULL;
> +     }
> +
> +#else

This causes a crash if path is an empty string.

>       if (path[0] != '/' && path[0] != '~') {
>               g_warning("\"%s\" is not an absolute path", path);
>               return NULL;
> @@ -70,7 +80,7 @@ char *parsePath(char *path)
>                               struct passwd *passwd = getpwnam(param->value);
>                               if (!passwd) {
>                                       g_warning("no such user %s",
> -                                               param->value);
> +                                           param->value);
>                                       return NULL;
>                               }
>  

???

> @@ -105,22 +115,21 @@ char *parsePath(char *path)
>                               *c = '/';
>  
>                       home = passwd->pw_dir;
> -             }
> +     } 

???

>  
>               return g_strconcat(home, path + pos, NULL);
> -     } else {
> +     } 
>  #endif
> +     else {
>               return g_strdup(path);
> -#ifndef WIN32
>       }
> -#endif
>  }
>  
>  int set_nonblocking(int fd)
>  {
> -#ifdef WIN32
> -     u_long val = 0;
> -
> +#ifdef _WIN32
> +     u_long val = 1;
> +
>       return ioctlsocket(fd, FIONBIO, &val) == 0 ? 0 : -1;
>  #else
>       int ret, flags;

Please explain.

> diff --git a/src/volume.c b/src/volume.c
> old mode 100644
> new mode 100755
> index 5eb0bd6..ac2add0
> --- a/src/volume.c
> +++ b/src/volume.c
> @@ -40,7 +40,11 @@
>  #define SW_VOLUME_STATE                         "sw_volume: "
>  
>  const struct audio_output_plugin *default_mixer;
> +#ifdef _WIN32
> +static int volume_mixer_type = VOLUME_MIXER_TYPE_SOFTWARE; /* hardware mixer 
> on windows not implemented yet!*/
> +#else
>  static int volume_mixer_type = VOLUME_MIXER_TYPE_HARDWARE;
> +#endif
>  static int volume_software_set = 100;
>  
>  void volume_finish(void)

This is wrong.  MPD should always attempt to find a hardware mixer.
If none is implemented, MPD should automatically fall back to
software.  This is a non-solution.

> diff --git a/src/zeroconf.c b/src/zeroconf.c
> old mode 100644
> new mode 100755
> index dd9790a..4f40419
> --- a/src/zeroconf.c
> +++ b/src/zeroconf.c
> @@ -22,7 +22,7 @@
>  #include "config.h"
>  
>  #include <glib.h>
> -
> +#ifdef HAVE_ZEROCONF
>  /* The default service name to publish
>   * (overridden by 'zeroconf_name' config parameter)
>   */
> @@ -71,3 +71,4 @@ void finishZeroconf(void)
>       bonjour_finish();
>  #endif
>  }
> +#endif /*HAVE_ZEROCONF*/
> \ No newline at end of file

This hunk is wrong, since zeroconf.c shouldn't be compiled at all if
HAVE_ZEROCONF is not defined.


Sorry Gunnary, your patch is completely unacceptable.  If there's one
thing I begged you then it's to *please* create single tiny patches
for separate changes, but it looks like you refused to obey that.
Most hunks have no effect except breaking MPD's code style by
inserting or deleting whitespaces.  There may be a few useful hunks
within that patch jungle, but it'll cost me less time to rewrite that
instead of trying to pick them from your patch.

Max

------------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It is the best place to buy or sell services for
just about anything Open Source.
http://p.sf.net/sfu/Xq1LFB
_______________________________________________
Musicpd-dev-team mailing list
Musicpd-dev-team@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/musicpd-dev-team

Reply via email to