From 779bf2b2d99c6bd468821dd1f177a94300593861 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:02:20 +0200 Subject: imported list.h from the Linux kernel sources linux/list.h is a nice doubly linked list library - it is lightweight and powerful at the same time. It will be useful later, when we begin to allocate client structures dynamically. Import it, and strip out all the stuff which we are not going to use. --- src/Makefile.am | 1 + src/dlist.h | 484 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 485 insertions(+) create mode 100644 src/dlist.h (limited to 'src') diff --git a/src/Makefile.am b/src/Makefile.am index 70306c125..63e17dda3 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -51,6 +51,7 @@ mpd_headers = \ inputStream_http_auth.h \ interface.h \ list.h \ + dlist.h \ listen.h \ log.h \ ls.h \ diff --git a/src/dlist.h b/src/dlist.h new file mode 100644 index 000000000..6123d7136 --- /dev/null +++ b/src/dlist.h @@ -0,0 +1,484 @@ +/* the Music Player Daemon (MPD) + * This project's homepage is: http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +/* + * This source was imported from the Linux kernel. It is licensed + * GPLv2 only. + * + */ + +#ifndef _LINUX_LIST_H +#define _LINUX_LIST_H + +/* + * These are non-NULL pointers that will result in page faults + * under normal circumstances, used to verify that nobody uses + * non-initialized list entries. + */ +#define LIST_POISON1 ((void *) 0x00100100) +#define LIST_POISON2 ((void *) 0x00200200) + +/* + * Simple doubly linked list implementation. + * + * Some of the internal functions ("__xxx") are useful when + * manipulating whole lists rather than single entries, as + * sometimes we already know the next/prev entries and we can + * generate better code by using them directly rather than + * using the generic single-entry routines. + */ + +struct list_head { + struct list_head *next, *prev; +}; + +#define LIST_HEAD_INIT(name) { &(name), &(name) } + +#define LIST_HEAD(name) \ + struct list_head name = LIST_HEAD_INIT(name) + +static inline void INIT_LIST_HEAD(struct list_head *list) +{ + list->next = list; + list->prev = list; +} + +/* + * Insert a new entry between two known consecutive entries. + * + * This is only for internal list manipulation where we know + * the prev/next entries already! + */ +#ifndef CONFIG_DEBUG_LIST +static inline void __list_add(struct list_head *new, + struct list_head *prev, + struct list_head *next) +{ + next->prev = new; + new->next = next; + new->prev = prev; + prev->next = new; +} +#else +extern void __list_add(struct list_head *new, + struct list_head *prev, + struct list_head *next); +#endif + +/** + * list_add - add a new entry + * @new: new entry to be added + * @head: list head to add it after + * + * Insert a new entry after the specified head. + * This is good for implementing stacks. + */ +#ifndef CONFIG_DEBUG_LIST +static inline void list_add(struct list_head *new, struct list_head *head) +{ + __list_add(new, head, head->next); +} +#else +extern void list_add(struct list_head *new, struct list_head *head); +#endif + + +/** + * list_add_tail - add a new entry + * @new: new entry to be added + * @head: list head to add it before + * + * Insert a new entry before the specified head. + * This is useful for implementing queues. + */ +static inline void list_add_tail(struct list_head *new, struct list_head *head) +{ + __list_add(new, head->prev, head); +} + +/* + * Delete a list entry by making the prev/next entries + * point to each other. + * + * This is only for internal list manipulation where we know + * the prev/next entries already! + */ +static inline void __list_del(struct list_head * prev, struct list_head * next) +{ + next->prev = prev; + prev->next = next; +} + +/** + * list_del - deletes entry from list. + * @entry: the element to delete from the list. + * Note: list_empty() on entry does not return true after this, the entry is + * in an undefined state. + */ +#ifndef CONFIG_DEBUG_LIST +static inline void list_del(struct list_head *entry) +{ + __list_del(entry->prev, entry->next); + entry->next = LIST_POISON1; + entry->prev = LIST_POISON2; +} +#else +extern void list_del(struct list_head *entry); +#endif + +/** + * list_replace - replace old entry by new one + * @old : the element to be replaced + * @new : the new element to insert + * + * If @old was empty, it will be overwritten. + */ +static inline void list_replace(struct list_head *old, + struct list_head *new) +{ + new->next = old->next; + new->next->prev = new; + new->prev = old->prev; + new->prev->next = new; +} + +static inline void list_replace_init(struct list_head *old, + struct list_head *new) +{ + list_replace(old, new); + INIT_LIST_HEAD(old); +} + +/** + * list_del_init - deletes entry from list and reinitialize it. + * @entry: the element to delete from the list. + */ +static inline void list_del_init(struct list_head *entry) +{ + __list_del(entry->prev, entry->next); + INIT_LIST_HEAD(entry); +} + +/** + * list_move - delete from one list and add as another's head + * @list: the entry to move + * @head: the head that will precede our entry + */ +static inline void list_move(struct list_head *list, struct list_head *head) +{ + __list_del(list->prev, list->next); + list_add(list, head); +} + +/** + * list_move_tail - delete from one list and add as another's tail + * @list: the entry to move + * @head: the head that will follow our entry + */ +static inline void list_move_tail(struct list_head *list, + struct list_head *head) +{ + __list_del(list->prev, list->next); + list_add_tail(list, head); +} + +/** + * list_is_last - tests whether @list is the last entry in list @head + * @list: the entry to test + * @head: the head of the list + */ +static inline int list_is_last(const struct list_head *list, + const struct list_head *head) +{ + return list->next == head; +} + +/** + * list_empty - tests whether a list is empty + * @head: the list to test. + */ +static inline int list_empty(const struct list_head *head) +{ + return head->next == head; +} + +/** + * list_empty_careful - tests whether a list is empty and not being modified + * @head: the list to test + * + * Description: + * tests whether a list is empty _and_ checks that no other CPU might be + * in the process of modifying either member (next or prev) + * + * NOTE: using list_empty_careful() without synchronization + * can only be safe if the only activity that can happen + * to the list entry is list_del_init(). Eg. it cannot be used + * if another CPU could re-list_add() it. + */ +static inline int list_empty_careful(const struct list_head *head) +{ + struct list_head *next = head->next; + return (next == head) && (next == head->prev); +} + +static inline void __list_splice(struct list_head *list, + struct list_head *head) +{ + struct list_head *first = list->next; + struct list_head *last = list->prev; + struct list_head *at = head->next; + + first->prev = head; + head->next = first; + + last->next = at; + at->prev = last; +} + +/** + * list_splice - join two lists + * @list: the new list to add. + * @head: the place to add it in the first list. + */ +static inline void list_splice(struct list_head *list, struct list_head *head) +{ + if (!list_empty(list)) + __list_splice(list, head); +} + +/** + * list_splice_init - join two lists and reinitialise the emptied list. + * @list: the new list to add. + * @head: the place to add it in the first list. + * + * The list at @list is reinitialised + */ +static inline void list_splice_init(struct list_head *list, + struct list_head *head) +{ + if (!list_empty(list)) { + __list_splice(list, head); + INIT_LIST_HEAD(list); + } +} + +/** + * list_entry - get the struct for this entry + * @ptr: the &struct list_head pointer. + * @type: the type of the struct this is embedded in. + * @member: the name of the list_struct within the struct. + */ +#define list_entry(ptr, type, member) \ + ((type*)ptr) + +/** + * list_first_entry - get the first element from a list + * @ptr: the list head to take the element from. + * @type: the type of the struct this is embedded in. + * @member: the name of the list_struct within the struct. + * + * Note, that list is expected to be not empty. + */ +#define list_first_entry(ptr, type, member) \ + list_entry((ptr)->next, type, member) + +/** + * list_for_each - iterate over a list + * @pos: the &struct list_head to use as a loop cursor. + * @head: the head for your list. + */ +#define list_for_each(pos, head) \ + for (pos = (head)->next; pos != (head); \ + pos = pos->next) + +/** + * __list_for_each - iterate over a list + * @pos: the &struct list_head to use as a loop cursor. + * @head: the head for your list. + * + * This variant differs from list_for_each() in that it's the + * simplest possible list iteration code, no prefetching is done. + * Use this for code that knows the list to be very short (empty + * or 1 entry) most of the time. + */ +#define __list_for_each(pos, head) \ + for (pos = (head)->next; pos != (head); pos = pos->next) + +/** + * list_for_each_prev - iterate over a list backwards + * @pos: the &struct list_head to use as a loop cursor. + * @head: the head for your list. + */ +#define list_for_each_prev(pos, head) \ + for (pos = (head)->prev; pos != (head); \ + pos = pos->prev) + +/** + * list_for_each_safe - iterate over a list safe against removal of list entry + * @pos: the &struct list_head to use as a loop cursor. + * @n: another &struct list_head to use as temporary storage + * @head: the head for your list. + */ +#define list_for_each_safe(pos, n, head) \ + for (pos = (head)->next, n = pos->next; pos != (head); \ + pos = n, n = pos->next) + +/** + * list_for_each_prev_safe - iterate over a list backwards safe against removal of list entry + * @pos: the &struct list_head to use as a loop cursor. + * @n: another &struct list_head to use as temporary storage + * @head: the head for your list. + */ +#define list_for_each_prev_safe(pos, n, head) \ + for (pos = (head)->prev, n = pos->prev; \ + pos != (head); \ + pos = n, n = pos->prev) + +/** + * list_for_each_entry - iterate over list of given type + * @pos: the type * to use as a loop cursor. + * @head: the head for your list. + * @member: the name of the list_struct within the struct. + */ +#define list_for_each_entry(pos, head, member) \ + for (pos = list_entry((head)->next, typeof(*pos), member); \ + &pos->member != (head); \ + pos = list_entry(pos->member.next, typeof(*pos), member)) + +/** + * list_for_each_entry_reverse - iterate backwards over list of given type. + * @pos: the type * to use as a loop cursor. + * @head: the head for your list. + * @member: the name of the list_struct within the struct. + */ +#define list_for_each_entry_reverse(pos, head, member) \ + for (pos = list_entry((head)->prev, typeof(*pos), member); \ + &pos->member != (head); \ + pos = list_entry(pos->member.prev, typeof(*pos), member)) + +/** + * list_prepare_entry - prepare a pos entry for use in list_for_each_entry_continue() + * @pos: the type * to use as a start point + * @head: the head of the list + * @member: the name of the list_struct within the struct. + * + * Prepares a pos entry for use as a start point in list_for_each_entry_continue(). + */ +#define list_prepare_entry(pos, head, member) \ + ((pos) ? : list_entry(head, typeof(*pos), member)) + +/** + * list_for_each_entry_continue - continue iteration over list of given type + * @pos: the type * to use as a loop cursor. + * @head: the head for your list. + * @member: the name of the list_struct within the struct. + * + * Continue to iterate over list of given type, continuing after + * the current position. + */ +#define list_for_each_entry_continue(pos, head, member) \ + for (pos = list_entry(pos->member.next, typeof(*pos), member); \ + &pos->member != (head); \ + pos = list_entry(pos->member.next, typeof(*pos), member)) + +/** + * list_for_each_entry_continue_reverse - iterate backwards from the given point + * @pos: the type * to use as a loop cursor. + * @head: the head for your list. + * @member: the name of the list_struct within the struct. + * + * Start to iterate over list of given type backwards, continuing after + * the current position. + */ +#define list_for_each_entry_continue_reverse(pos, head, member) \ + for (pos = list_entry(pos->member.prev, typeof(*pos), member); \ + &pos->member != (head); \ + pos = list_entry(pos->member.prev, typeof(*pos), member)) + +/** + * list_for_each_entry_from - iterate over list of given type from the current point + * @pos: the type * to use as a loop cursor. + * @head: the head for your list. + * @member: the name of the list_struct within the struct. + * + * Iterate over list of given type, continuing from current position. + */ +#define list_for_each_entry_from(pos, head, member) \ + for (; &pos->member != (head); \ + pos = list_entry(pos->member.next, typeof(*pos), member)) + +/** + * list_for_each_entry_safe - iterate over list of given type safe against removal of list entry + * @pos: the type * to use as a loop cursor. + * @n: another type * to use as temporary storage + * @head: the head for your list. + * @member: the name of the list_struct within the struct. + */ +#define list_for_each_entry_safe(pos, n, head, member) \ + for (pos = list_entry((head)->next, typeof(*pos), member), \ + n = list_entry(pos->member.next, typeof(*pos), member); \ + &pos->member != (head); \ + pos = n, n = list_entry(n->member.next, typeof(*n), member)) + +/** + * list_for_each_entry_safe_continue + * @pos: the type * to use as a loop cursor. + * @n: another type * to use as temporary storage + * @head: the head for your list. + * @member: the name of the list_struct within the struct. + * + * Iterate over list of given type, continuing after current point, + * safe against removal of list entry. + */ +#define list_for_each_entry_safe_continue(pos, n, head, member) \ + for (pos = list_entry(pos->member.next, typeof(*pos), member), \ + n = list_entry(pos->member.next, typeof(*pos), member); \ + &pos->member != (head); \ + pos = n, n = list_entry(n->member.next, typeof(*n), member)) + +/** + * list_for_each_entry_safe_from + * @pos: the type * to use as a loop cursor. + * @n: another type * to use as temporary storage + * @head: the head for your list. + * @member: the name of the list_struct within the struct. + * + * Iterate over list of given type from current point, safe against + * removal of list entry. + */ +#define list_for_each_entry_safe_from(pos, n, head, member) \ + for (n = list_entry(pos->member.next, typeof(*pos), member); \ + &pos->member != (head); \ + pos = n, n = list_entry(n->member.next, typeof(*n), member)) + +/** + * list_for_each_entry_safe_reverse + * @pos: the type * to use as a loop cursor. + * @n: another type * to use as temporary storage + * @head: the head for your list. + * @member: the name of the list_struct within the struct. + * + * Iterate backwards over list of given type, safe against removal + * of list entry. + */ +#define list_for_each_entry_safe_reverse(pos, n, head, member) \ + for (pos = list_entry((head)->prev, typeof(*pos), member), \ + n = list_entry(pos->member.prev, typeof(*pos), member); \ + &pos->member != (head); \ + pos = n, n = list_entry(n->member.prev, typeof(*n), member)) + +#endif -- cgit v1.2.3 From bd801d6d2ed4b71255dc314f56b8c87220b29853 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:02:43 +0200 Subject: renamed interface.c to client.c I don't believe "interface" is a good name for something like "connection by a client to MPD", let's call it "client". This is the first patch in the series which changes the name, beginning with the file name. --- src/Makefile.am | 4 +- src/client.c | 794 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/client.h | 32 +++ src/directory.c | 2 +- src/interface.c | 794 -------------------------------------------------------- src/interface.h | 32 --- src/listen.c | 2 +- src/main.c | 2 +- src/myfprintf.c | 2 +- 9 files changed, 832 insertions(+), 832 deletions(-) create mode 100644 src/client.c create mode 100644 src/client.h delete mode 100644 src/interface.c delete mode 100644 src/interface.h (limited to 'src') diff --git a/src/Makefile.am b/src/Makefile.am index 63e17dda3..66eea5fff 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -49,7 +49,7 @@ mpd_headers = \ inputStream_file.h \ inputStream_http.h \ inputStream_http_auth.h \ - interface.h \ + client.h \ list.h \ dlist.h \ listen.h \ @@ -112,7 +112,7 @@ mpd_SOURCES = \ inputStream.c \ inputStream_file.c \ inputStream_http.c \ - interface.c \ + client.c \ ioops.c \ list.c \ listen.c \ diff --git a/src/client.c b/src/client.c new file mode 100644 index 000000000..a6ca85497 --- /dev/null +++ b/src/client.c @@ -0,0 +1,794 @@ +/* the Music Player Daemon (MPD) + * Copyright (C) 2003-2007 by Warren Dukes (warren.dukes@gmail.com) + * This project's homepage is: http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#include "client.h" +#include "command.h" +#include "conf.h" +#include "log.h" +#include "listen.h" +#include "permission.h" +#include "sllist.h" +#include "utils.h" +#include "ioops.h" +#include "myfprintf.h" +#include "os_compat.h" +#include "main_notify.h" + +#include "../config.h" + +#define GREETING "OK MPD " PROTOCOL_VERSION "\n" + +#define INTERFACE_MAX_BUFFER_LENGTH (40960) +#define INTERFACE_LIST_MODE_BEGIN "command_list_begin" +#define INTERFACE_LIST_OK_MODE_BEGIN "command_list_ok_begin" +#define INTERFACE_LIST_MODE_END "command_list_end" +#define INTERFACE_DEFAULT_OUT_BUFFER_SIZE (4096) +#define INTERFACE_TIMEOUT_DEFAULT (60) +#define INTERFACE_MAX_CONNECTIONS_DEFAULT (10) +#define INTERFACE_MAX_COMMAND_LIST_DEFAULT (2048*1024) +#define INTERFACE_MAX_OUTPUT_BUFFER_SIZE_DEFAULT (8192*1024) + +/* set this to zero to indicate we have no possible interfaces */ +static unsigned int interface_max_connections; /*INTERFACE_MAX_CONNECTIONS_DEFAULT; */ +static int interface_timeout = INTERFACE_TIMEOUT_DEFAULT; +static size_t interface_max_command_list_size = + INTERFACE_MAX_COMMAND_LIST_DEFAULT; +static size_t interface_max_output_buffer_size = + INTERFACE_MAX_OUTPUT_BUFFER_SIZE_DEFAULT; + +/* maybe make conf option for this, or... 32 might be good enough */ +static long int interface_list_cache_size = 32; + +/* shared globally between all interfaces: */ +static struct strnode *list_cache; +static struct strnode *list_cache_head; +static struct strnode *list_cache_tail; + +typedef struct _Interface { + char buffer[INTERFACE_MAX_BUFFER_LENGTH]; + size_t bufferLength; + size_t bufferPos; + int fd; /* file descriptor */ + int permission; + time_t lastTime; + struct strnode *cmd_list; /* for when in list mode */ + struct strnode *cmd_list_tail; /* for when in list mode */ + int cmd_list_OK; /* print OK after each command execution */ + size_t cmd_list_size; /* mem cmd_list consumes */ + int cmd_list_dup; /* has the cmd_list been copied to private space? */ + struct sllnode *deferred_send; /* for output if client is slow */ + size_t deferred_bytes; /* mem deferred_send consumes */ + int expired; /* set whether this interface should be closed on next + check of old interfaces */ + unsigned int num; /* interface number */ + + char *send_buf; + size_t send_buf_used; /* bytes used this instance */ + size_t send_buf_size; /* bytes usable this instance */ + size_t send_buf_alloc; /* bytes actually allocated */ +} Interface; + +static Interface *interfaces; + +static void flushInterfaceBuffer(Interface * interface); + +static void printInterfaceOutBuffer(Interface * interface); + +#ifdef SO_SNDBUF +static size_t get_default_snd_buf_size(Interface * interface) +{ + int new_size; + socklen_t sockOptLen = sizeof(int); + + if (getsockopt(interface->fd, SOL_SOCKET, SO_SNDBUF, + (char *)&new_size, &sockOptLen) < 0) { + DEBUG("problem getting sockets send buffer size\n"); + return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; + } + if (new_size > 0) + return (size_t)new_size; + DEBUG("sockets send buffer size is not positive\n"); + return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; +} +#else /* !SO_SNDBUF */ +static size_t get_default_snd_buf_size(Interface * interface) +{ + return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; +} +#endif /* !SO_SNDBUF */ + +static void set_send_buf_size(Interface * interface) +{ + size_t new_size = get_default_snd_buf_size(interface); + if (interface->send_buf_size != new_size) { + interface->send_buf_size = new_size; + /* don't resize to get smaller, only bigger */ + if (interface->send_buf_alloc < new_size) { + if (interface->send_buf) + free(interface->send_buf); + interface->send_buf = xmalloc(new_size); + interface->send_buf_alloc = new_size; + } + } +} + +static void openInterface(Interface * interface, int fd) +{ + assert(interface->fd < 0); + + interface->cmd_list_size = 0; + interface->cmd_list_dup = 0; + interface->cmd_list_OK = -1; + interface->bufferLength = 0; + interface->bufferPos = 0; + interface->fd = fd; + set_nonblocking(fd); + interface->lastTime = time(NULL); + interface->cmd_list = NULL; + interface->cmd_list_tail = NULL; + interface->deferred_send = NULL; + interface->expired = 0; + interface->deferred_bytes = 0; + interface->send_buf_used = 0; + + interface->permission = getDefaultPermissions(); + set_send_buf_size(interface); + + xwrite(fd, GREETING, strlen(GREETING)); +} + +static void free_cmd_list(struct strnode *list) +{ + struct strnode *tmp = list; + + while (tmp) { + struct strnode *next = tmp->next; + if (tmp >= list_cache_head && tmp <= list_cache_tail) { + /* inside list_cache[] array */ + tmp->data = NULL; + tmp->next = NULL; + } else + free(tmp); + tmp = next; + } +} + +static void cmd_list_clone(Interface * interface) +{ + struct strnode *new = dup_strlist(interface->cmd_list); + free_cmd_list(interface->cmd_list); + interface->cmd_list = new; + interface->cmd_list_dup = 1; + + /* new tail */ + while (new && new->next) + new = new->next; + interface->cmd_list_tail = new; +} + +static void new_cmd_list_ptr(Interface * interface, char *s, const int size) +{ + int i; + struct strnode *new; + + if (!interface->cmd_list_dup) { + for (i = interface_list_cache_size - 1; i >= 0; --i) { + if (list_cache[i].data) + continue; + new = &(list_cache[i]); + new->data = s; + /* implied in free_cmd_list() and init: */ + /* last->next->next = NULL; */ + goto out; + } + } + + /* allocate from the heap */ + new = interface->cmd_list_dup ? new_strnode_dup(s, size) + : new_strnode(s); +out: + if (interface->cmd_list) { + interface->cmd_list_tail->next = new; + interface->cmd_list_tail = new; + } else + interface->cmd_list = interface->cmd_list_tail = new; +} + +static void closeInterface(Interface * interface) +{ + struct sllnode *buf; + if (interface->fd < 0) + return; + xclose(interface->fd); + interface->fd = -1; + + if (interface->cmd_list) { + free_cmd_list(interface->cmd_list); + interface->cmd_list = NULL; + } + + if ((buf = interface->deferred_send)) { + do { + struct sllnode *prev = buf; + buf = buf->next; + free(prev); + } while (buf); + interface->deferred_send = NULL; + } + + SECURE("interface %i: closed\n", interface->num); +} + +void openAInterface(int fd, const struct sockaddr *addr) +{ + unsigned int i; + + for (i = 0; i < interface_max_connections + && interfaces[i].fd >= 0; i++) /* nothing */ ; + + if (i == interface_max_connections) { + ERROR("Max Connections Reached!\n"); + xclose(fd); + } else { + const char *hostname; + switch (addr->sa_family) { +#ifdef HAVE_TCP + case AF_INET: + hostname = (const char *)inet_ntoa(((const struct sockaddr_in *) + addr)->sin_addr); + if (!hostname) + hostname = "error getting ipv4 address"; + break; +#ifdef HAVE_IPV6 + case AF_INET6: + { + static char host[INET6_ADDRSTRLEN + 1]; + memset(host, 0, INET6_ADDRSTRLEN + 1); + if (inet_ntop(AF_INET6, (const void *) + &(((const struct sockaddr_in6 *)addr)-> + sin6_addr), host, + INET6_ADDRSTRLEN)) { + hostname = (const char *)host; + } else { + hostname = "error getting ipv6 address"; + } + } + break; +#endif +#endif /* HAVE_TCP */ +#ifdef HAVE_UN + case AF_UNIX: + hostname = "local connection"; + break; +#endif /* HAVE_UN */ + default: + hostname = "unknown"; + } + SECURE("interface %i: opened from %s\n", i, hostname); + openInterface(&(interfaces[i]), fd); + } +} + +static int processLineOfInput(Interface * interface) +{ + int ret = 1; + char *line = interface->buffer + interface->bufferPos; + + if (interface->cmd_list_OK >= 0) { + if (strcmp(line, INTERFACE_LIST_MODE_END) == 0) { + DEBUG("interface %i: process command " + "list\n", interface->num); + ret = processListOfCommands(interface->fd, + &(interface->permission), + &(interface->expired), + interface->cmd_list_OK, + interface->cmd_list); + DEBUG("interface %i: process command " + "list returned %i\n", interface->num, ret); + if (ret == 0) + commandSuccess(interface->fd); + else if (ret == COMMAND_RETURN_CLOSE + || interface->expired) + closeInterface(interface); + + printInterfaceOutBuffer(interface); + free_cmd_list(interface->cmd_list); + interface->cmd_list = NULL; + interface->cmd_list_OK = -1; + } else { + size_t len = strlen(line) + 1; + interface->cmd_list_size += len; + if (interface->cmd_list_size > + interface_max_command_list_size) { + ERROR("interface %i: command " + "list size (%lu) is " + "larger than the max " + "(%lu)\n", + interface->num, + (unsigned long)interface->cmd_list_size, + (unsigned long) + interface_max_command_list_size); + closeInterface(interface); + ret = COMMAND_RETURN_CLOSE; + } else + new_cmd_list_ptr(interface, line, len); + } + } else { + if (strcmp(line, INTERFACE_LIST_MODE_BEGIN) == 0) { + interface->cmd_list_OK = 0; + ret = 1; + } else if (strcmp(line, INTERFACE_LIST_OK_MODE_BEGIN) == 0) { + interface->cmd_list_OK = 1; + ret = 1; + } else { + DEBUG("interface %i: process command \"%s\"\n", + interface->num, line); + ret = processCommand(interface->fd, + &(interface->permission), line); + DEBUG("interface %i: command returned %i\n", + interface->num, ret); + if (ret == 0) + commandSuccess(interface->fd); + else if (ret == COMMAND_RETURN_CLOSE + || interface->expired) { + closeInterface(interface); + } + printInterfaceOutBuffer(interface); + } + } + + return ret; +} + +static int processBytesRead(Interface * interface, int bytesRead) +{ + int ret = 0; + char *buf_tail = &(interface->buffer[interface->bufferLength - 1]); + + while (bytesRead > 0) { + interface->bufferLength++; + bytesRead--; + buf_tail++; + if (*buf_tail == '\n') { + *buf_tail = '\0'; + if (interface->bufferLength > interface->bufferPos) { + if (*(buf_tail - 1) == '\r') + *(buf_tail - 1) = '\0'; + } + ret = processLineOfInput(interface); + if (interface->expired) + return ret; + interface->bufferPos = interface->bufferLength; + } + if (interface->bufferLength == INTERFACE_MAX_BUFFER_LENGTH) { + if (interface->bufferPos == 0) { + ERROR("interface %i: buffer overflow\n", + interface->num); + closeInterface(interface); + return 1; + } + if (interface->cmd_list_OK >= 0 && + interface->cmd_list && + !interface->cmd_list_dup) + cmd_list_clone(interface); + assert(interface->bufferLength >= interface->bufferPos + && "bufferLength >= bufferPos"); + interface->bufferLength -= interface->bufferPos; + memmove(interface->buffer, + interface->buffer + interface->bufferPos, + interface->bufferLength); + interface->bufferPos = 0; + } + if (ret == COMMAND_RETURN_KILL || ret == COMMAND_RETURN_CLOSE) { + return ret; + } + + } + + return ret; +} + +static int interfaceReadInput(Interface * interface) +{ + int bytesRead; + + bytesRead = read(interface->fd, + interface->buffer + interface->bufferLength, + INTERFACE_MAX_BUFFER_LENGTH - interface->bufferLength); + + if (bytesRead > 0) + return processBytesRead(interface, bytesRead); + else if (bytesRead == 0 || (bytesRead < 0 && errno != EINTR)) { + closeInterface(interface); + } else + return 0; + + return 1; +} + +static void addInterfacesReadyToReadAndListenSocketToFdSet(fd_set * fds, + int *fdmax) +{ + unsigned int i; + + FD_ZERO(fds); + addListenSocketsToFdSet(fds, fdmax); + + for (i = 0; i < interface_max_connections; i++) { + if (interfaces[i].fd >= 0 && !interfaces[i].expired + && !interfaces[i].deferred_send) { + FD_SET(interfaces[i].fd, fds); + if (*fdmax < interfaces[i].fd) + *fdmax = interfaces[i].fd; + } + } +} + +static void addInterfacesForBufferFlushToFdSet(fd_set * fds, int *fdmax) +{ + unsigned int i; + + FD_ZERO(fds); + + for (i = 0; i < interface_max_connections; i++) { + if (interfaces[i].fd >= 0 && !interfaces[i].expired + && interfaces[i].deferred_send) { + FD_SET(interfaces[i].fd, fds); + if (*fdmax < interfaces[i].fd) + *fdmax = interfaces[i].fd; + } + } +} + +static void closeNextErroredInterface(void) +{ + fd_set fds; + struct timeval tv; + unsigned int i; + + tv.tv_sec = 0; + tv.tv_usec = 0; + + for (i = 0; i < interface_max_connections; i++) { + if (interfaces[i].fd >= 0) { + FD_ZERO(&fds); + FD_SET(interfaces[i].fd, &fds); + if (select(interfaces[i].fd + 1, + &fds, NULL, NULL, &tv) < 0) { + closeInterface(&interfaces[i]); + return; + } + } + } +} + +int doIOForInterfaces(void) +{ + fd_set rfds; + fd_set wfds; + fd_set efds; + unsigned int i; + int selret; + int fdmax; + + while (1) { + fdmax = 0; + + FD_ZERO( &efds ); + addInterfacesReadyToReadAndListenSocketToFdSet(&rfds, &fdmax); + addInterfacesForBufferFlushToFdSet(&wfds, &fdmax); + + registered_IO_add_fds(&fdmax, &rfds, &wfds, &efds); + + selret = select(fdmax + 1, &rfds, &wfds, &efds, NULL); + if (selret < 0 && errno == EINTR) + break; + + registered_IO_consume_fds(&selret, &rfds, &wfds, &efds); + + if (selret == 0) + break; + + if (selret < 0) { + closeNextErroredInterface(); + continue; + } + + getConnections(&rfds); + + for (i = 0; i < interface_max_connections; i++) { + if (interfaces[i].fd >= 0 + && FD_ISSET(interfaces[i].fd, &rfds)) { + if (COMMAND_RETURN_KILL == + interfaceReadInput(&(interfaces[i]))) { + return COMMAND_RETURN_KILL; + } + interfaces[i].lastTime = time(NULL); + } + if (interfaces[i].fd >= 0 + && FD_ISSET(interfaces[i].fd, &wfds)) { + flushInterfaceBuffer(&interfaces[i]); + interfaces[i].lastTime = time(NULL); + } + } + + break; + } + + return 1; +} + +void initInterfaces(void) +{ + unsigned int i; + char *test; + ConfigParam *param; + + param = getConfigParam(CONF_CONN_TIMEOUT); + + if (param) { + interface_timeout = strtol(param->value, &test, 10); + if (*test != '\0' || interface_timeout <= 0) { + FATAL("connection timeout \"%s\" is not a positive " + "integer, line %i\n", CONF_CONN_TIMEOUT, + param->line); + } + } + + param = getConfigParam(CONF_MAX_CONN); + + if (param) { + interface_max_connections = strtol(param->value, &test, 10); + if (*test != '\0' || interface_max_connections <= 0) { + FATAL("max connections \"%s\" is not a positive integer" + ", line %i\n", param->value, param->line); + } + } else + interface_max_connections = INTERFACE_MAX_CONNECTIONS_DEFAULT; + + param = getConfigParam(CONF_MAX_COMMAND_LIST_SIZE); + + if (param) { + long tmp = strtol(param->value, &test, 10); + if (*test != '\0' || tmp <= 0) { + FATAL("max command list size \"%s\" is not a positive " + "integer, line %i\n", param->value, param->line); + } + interface_max_command_list_size = tmp * 1024; + } + + param = getConfigParam(CONF_MAX_OUTPUT_BUFFER_SIZE); + + if (param) { + long tmp = strtol(param->value, &test, 10); + if (*test != '\0' || tmp <= 0) { + FATAL("max output buffer size \"%s\" is not a positive " + "integer, line %i\n", param->value, param->line); + } + interface_max_output_buffer_size = tmp * 1024; + } + + interfaces = xmalloc(sizeof(Interface) * interface_max_connections); + + list_cache = xcalloc(interface_list_cache_size, sizeof(struct strnode)); + list_cache_head = &(list_cache[0]); + list_cache_tail = &(list_cache[interface_list_cache_size - 1]); + + for (i = 0; i < interface_max_connections; i++) { + interfaces[i].fd = -1; + interfaces[i].send_buf = NULL; + interfaces[i].send_buf_size = 0; + interfaces[i].send_buf_alloc = 0; + interfaces[i].num = i; + } +} + +static void closeAllInterfaces(void) +{ + unsigned int i; + + for (i = 0; i < interface_max_connections; i++) { + if (interfaces[i].fd >= 0) + closeInterface(&(interfaces[i])); + if (interfaces[i].send_buf) + free(interfaces[i].send_buf); + } + free(list_cache); +} + +void freeAllInterfaces(void) +{ + closeAllInterfaces(); + + free(interfaces); + + interface_max_connections = 0; +} + +void closeOldInterfaces(void) +{ + unsigned int i; + + for (i = 0; i < interface_max_connections; i++) { + if (interfaces[i].fd >= 0) { + if (interfaces[i].expired) { + DEBUG("interface %i: expired\n", i); + closeInterface(&(interfaces[i])); + } else if (time(NULL) - interfaces[i].lastTime > + interface_timeout) { + DEBUG("interface %i: timeout\n", i); + closeInterface(&(interfaces[i])); + } + } + } +} + +static void flushInterfaceBuffer(Interface * interface) +{ + struct sllnode *buf; + ssize_t ret = 0; + + buf = interface->deferred_send; + while (buf) { + ret = write(interface->fd, buf->data, buf->size); + if (ret < 0) + break; + else if ((size_t)ret < buf->size) { + assert(interface->deferred_bytes >= (size_t)ret); + interface->deferred_bytes -= ret; + buf->data = (char *)buf->data + ret; + buf->size -= ret; + } else { + struct sllnode *tmp = buf; + size_t decr = (buf->size + sizeof(struct sllnode)); + + assert(interface->deferred_bytes >= decr); + interface->deferred_bytes -= decr; + buf = buf->next; + free(tmp); + interface->deferred_send = buf; + } + interface->lastTime = time(NULL); + } + + if (!interface->deferred_send) { + DEBUG("interface %i: buffer empty %lu\n", interface->num, + (unsigned long)interface->deferred_bytes); + assert(interface->deferred_bytes == 0); + } else if (ret < 0 && errno != EAGAIN && errno != EINTR) { + /* cause interface to close */ + DEBUG("interface %i: problems flushing buffer\n", + interface->num); + buf = interface->deferred_send; + do { + struct sllnode *prev = buf; + buf = buf->next; + free(prev); + } while (buf); + interface->deferred_send = NULL; + interface->deferred_bytes = 0; + interface->expired = 1; + } +} + +int interfacePrintWithFD(int fd, const char *buffer, size_t buflen) +{ + static unsigned int i; + size_t copylen; + Interface *interface; + + assert(fd >= 0); + + if (i >= interface_max_connections || + interfaces[i].fd < 0 || interfaces[i].fd != fd) { + for (i = 0; i < interface_max_connections; i++) { + if (interfaces[i].fd == fd) + break; + } + if (i == interface_max_connections) + return -1; + } + + /* if fd isn't found or interfaces is going to be closed, do nothing */ + if (interfaces[i].expired) + return 0; + + interface = interfaces + i; + + while (buflen > 0 && !interface->expired) { + size_t left; + + assert(interface->send_buf_size >= interface->send_buf_used); + left = interface->send_buf_size - interface->send_buf_used; + + copylen = buflen > left ? left : buflen; + memcpy(interface->send_buf + interface->send_buf_used, buffer, + copylen); + buflen -= copylen; + interface->send_buf_used += copylen; + buffer += copylen; + if (interface->send_buf_used >= interface->send_buf_size) + printInterfaceOutBuffer(interface); + } + + return 0; +} + +static void printInterfaceOutBuffer(Interface * interface) +{ + ssize_t ret; + struct sllnode *buf; + + if (interface->fd < 0 || interface->expired || + !interface->send_buf_used) + return; + + if ((buf = interface->deferred_send)) { + interface->deferred_bytes += sizeof(struct sllnode) + + interface->send_buf_used; + if (interface->deferred_bytes > + interface_max_output_buffer_size) { + ERROR("interface %i: output buffer size (%lu) is " + "larger than the max (%lu)\n", + interface->num, + (unsigned long)interface->deferred_bytes, + (unsigned long)interface_max_output_buffer_size); + /* cause interface to close */ + interface->expired = 1; + do { + struct sllnode *prev = buf; + buf = buf->next; + free(prev); + } while (buf); + interface->deferred_send = NULL; + interface->deferred_bytes = 0; + } else { + while (buf->next) + buf = buf->next; + buf->next = new_sllnode(interface->send_buf, + interface->send_buf_used); + } + } else { + if ((ret = write(interface->fd, interface->send_buf, + interface->send_buf_used)) < 0) { + if (errno == EAGAIN || errno == EINTR) { + interface->deferred_send = + new_sllnode(interface->send_buf, + interface->send_buf_used); + } else { + DEBUG("interface %i: problems writing\n", + interface->num); + interface->expired = 1; + return; + } + } else if ((size_t)ret < interface->send_buf_used) { + interface->deferred_send = + new_sllnode(interface->send_buf + ret, + interface->send_buf_used - ret); + } + if (interface->deferred_send) { + DEBUG("interface %i: buffer created\n", interface->num); + interface->deferred_bytes = + interface->deferred_send->size + + sizeof(struct sllnode); + } + } + + interface->send_buf_used = 0; +} + diff --git a/src/client.h b/src/client.h new file mode 100644 index 000000000..c83381319 --- /dev/null +++ b/src/client.h @@ -0,0 +1,32 @@ +/* the Music Player Daemon (MPD) + * Copyright (C) 2003-2007 by Warren Dukes (warren.dukes@gmail.com) + * This project's homepage is: http://www.musicpd.org + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef INTERFACE_H +#define INTERFACE_H + +#include "os_compat.h" + +void initInterfaces(void); +void openAInterface(int fd, const struct sockaddr *addr); +void freeAllInterfaces(void); +void closeOldInterfaces(void); +int interfacePrintWithFD(int fd, const char *buffer, size_t len); + +int doIOForInterfaces(void); + +#endif diff --git a/src/directory.c b/src/directory.c index 94a55d664..6f3409356 100644 --- a/src/directory.c +++ b/src/directory.c @@ -20,7 +20,7 @@ #include "command.h" #include "conf.h" -#include "interface.h" +#include "client.h" #include "listen.h" #include "log.h" #include "ls.h" diff --git a/src/interface.c b/src/interface.c deleted file mode 100644 index 83e0084b5..000000000 --- a/src/interface.c +++ /dev/null @@ -1,794 +0,0 @@ -/* the Music Player Daemon (MPD) - * Copyright (C) 2003-2007 by Warren Dukes (warren.dukes@gmail.com) - * This project's homepage is: http://www.musicpd.org - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - */ - -#include "interface.h" -#include "command.h" -#include "conf.h" -#include "log.h" -#include "listen.h" -#include "permission.h" -#include "sllist.h" -#include "utils.h" -#include "ioops.h" -#include "myfprintf.h" -#include "os_compat.h" -#include "main_notify.h" - -#include "../config.h" - -#define GREETING "OK MPD " PROTOCOL_VERSION "\n" - -#define INTERFACE_MAX_BUFFER_LENGTH (40960) -#define INTERFACE_LIST_MODE_BEGIN "command_list_begin" -#define INTERFACE_LIST_OK_MODE_BEGIN "command_list_ok_begin" -#define INTERFACE_LIST_MODE_END "command_list_end" -#define INTERFACE_DEFAULT_OUT_BUFFER_SIZE (4096) -#define INTERFACE_TIMEOUT_DEFAULT (60) -#define INTERFACE_MAX_CONNECTIONS_DEFAULT (10) -#define INTERFACE_MAX_COMMAND_LIST_DEFAULT (2048*1024) -#define INTERFACE_MAX_OUTPUT_BUFFER_SIZE_DEFAULT (8192*1024) - -/* set this to zero to indicate we have no possible interfaces */ -static unsigned int interface_max_connections; /*INTERFACE_MAX_CONNECTIONS_DEFAULT; */ -static int interface_timeout = INTERFACE_TIMEOUT_DEFAULT; -static size_t interface_max_command_list_size = - INTERFACE_MAX_COMMAND_LIST_DEFAULT; -static size_t interface_max_output_buffer_size = - INTERFACE_MAX_OUTPUT_BUFFER_SIZE_DEFAULT; - -/* maybe make conf option for this, or... 32 might be good enough */ -static long int interface_list_cache_size = 32; - -/* shared globally between all interfaces: */ -static struct strnode *list_cache; -static struct strnode *list_cache_head; -static struct strnode *list_cache_tail; - -typedef struct _Interface { - char buffer[INTERFACE_MAX_BUFFER_LENGTH]; - size_t bufferLength; - size_t bufferPos; - int fd; /* file descriptor */ - int permission; - time_t lastTime; - struct strnode *cmd_list; /* for when in list mode */ - struct strnode *cmd_list_tail; /* for when in list mode */ - int cmd_list_OK; /* print OK after each command execution */ - size_t cmd_list_size; /* mem cmd_list consumes */ - int cmd_list_dup; /* has the cmd_list been copied to private space? */ - struct sllnode *deferred_send; /* for output if client is slow */ - size_t deferred_bytes; /* mem deferred_send consumes */ - int expired; /* set whether this interface should be closed on next - check of old interfaces */ - unsigned int num; /* interface number */ - - char *send_buf; - size_t send_buf_used; /* bytes used this instance */ - size_t send_buf_size; /* bytes usable this instance */ - size_t send_buf_alloc; /* bytes actually allocated */ -} Interface; - -static Interface *interfaces; - -static void flushInterfaceBuffer(Interface * interface); - -static void printInterfaceOutBuffer(Interface * interface); - -#ifdef SO_SNDBUF -static size_t get_default_snd_buf_size(Interface * interface) -{ - int new_size; - socklen_t sockOptLen = sizeof(int); - - if (getsockopt(interface->fd, SOL_SOCKET, SO_SNDBUF, - (char *)&new_size, &sockOptLen) < 0) { - DEBUG("problem getting sockets send buffer size\n"); - return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; - } - if (new_size > 0) - return (size_t)new_size; - DEBUG("sockets send buffer size is not positive\n"); - return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; -} -#else /* !SO_SNDBUF */ -static size_t get_default_snd_buf_size(Interface * interface) -{ - return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; -} -#endif /* !SO_SNDBUF */ - -static void set_send_buf_size(Interface * interface) -{ - size_t new_size = get_default_snd_buf_size(interface); - if (interface->send_buf_size != new_size) { - interface->send_buf_size = new_size; - /* don't resize to get smaller, only bigger */ - if (interface->send_buf_alloc < new_size) { - if (interface->send_buf) - free(interface->send_buf); - interface->send_buf = xmalloc(new_size); - interface->send_buf_alloc = new_size; - } - } -} - -static void openInterface(Interface * interface, int fd) -{ - assert(interface->fd < 0); - - interface->cmd_list_size = 0; - interface->cmd_list_dup = 0; - interface->cmd_list_OK = -1; - interface->bufferLength = 0; - interface->bufferPos = 0; - interface->fd = fd; - set_nonblocking(fd); - interface->lastTime = time(NULL); - interface->cmd_list = NULL; - interface->cmd_list_tail = NULL; - interface->deferred_send = NULL; - interface->expired = 0; - interface->deferred_bytes = 0; - interface->send_buf_used = 0; - - interface->permission = getDefaultPermissions(); - set_send_buf_size(interface); - - xwrite(fd, GREETING, strlen(GREETING)); -} - -static void free_cmd_list(struct strnode *list) -{ - struct strnode *tmp = list; - - while (tmp) { - struct strnode *next = tmp->next; - if (tmp >= list_cache_head && tmp <= list_cache_tail) { - /* inside list_cache[] array */ - tmp->data = NULL; - tmp->next = NULL; - } else - free(tmp); - tmp = next; - } -} - -static void cmd_list_clone(Interface * interface) -{ - struct strnode *new = dup_strlist(interface->cmd_list); - free_cmd_list(interface->cmd_list); - interface->cmd_list = new; - interface->cmd_list_dup = 1; - - /* new tail */ - while (new && new->next) - new = new->next; - interface->cmd_list_tail = new; -} - -static void new_cmd_list_ptr(Interface * interface, char *s, const int size) -{ - int i; - struct strnode *new; - - if (!interface->cmd_list_dup) { - for (i = interface_list_cache_size - 1; i >= 0; --i) { - if (list_cache[i].data) - continue; - new = &(list_cache[i]); - new->data = s; - /* implied in free_cmd_list() and init: */ - /* last->next->next = NULL; */ - goto out; - } - } - - /* allocate from the heap */ - new = interface->cmd_list_dup ? new_strnode_dup(s, size) - : new_strnode(s); -out: - if (interface->cmd_list) { - interface->cmd_list_tail->next = new; - interface->cmd_list_tail = new; - } else - interface->cmd_list = interface->cmd_list_tail = new; -} - -static void closeInterface(Interface * interface) -{ - struct sllnode *buf; - if (interface->fd < 0) - return; - xclose(interface->fd); - interface->fd = -1; - - if (interface->cmd_list) { - free_cmd_list(interface->cmd_list); - interface->cmd_list = NULL; - } - - if ((buf = interface->deferred_send)) { - do { - struct sllnode *prev = buf; - buf = buf->next; - free(prev); - } while (buf); - interface->deferred_send = NULL; - } - - SECURE("interface %i: closed\n", interface->num); -} - -void openAInterface(int fd, const struct sockaddr *addr) -{ - unsigned int i; - - for (i = 0; i < interface_max_connections - && interfaces[i].fd >= 0; i++) /* nothing */ ; - - if (i == interface_max_connections) { - ERROR("Max Connections Reached!\n"); - xclose(fd); - } else { - const char *hostname; - switch (addr->sa_family) { -#ifdef HAVE_TCP - case AF_INET: - hostname = (const char *)inet_ntoa(((const struct sockaddr_in *) - addr)->sin_addr); - if (!hostname) - hostname = "error getting ipv4 address"; - break; -#ifdef HAVE_IPV6 - case AF_INET6: - { - static char host[INET6_ADDRSTRLEN + 1]; - memset(host, 0, INET6_ADDRSTRLEN + 1); - if (inet_ntop(AF_INET6, (const void *) - &(((const struct sockaddr_in6 *)addr)-> - sin6_addr), host, - INET6_ADDRSTRLEN)) { - hostname = (const char *)host; - } else { - hostname = "error getting ipv6 address"; - } - } - break; -#endif -#endif /* HAVE_TCP */ -#ifdef HAVE_UN - case AF_UNIX: - hostname = "local connection"; - break; -#endif /* HAVE_UN */ - default: - hostname = "unknown"; - } - SECURE("interface %i: opened from %s\n", i, hostname); - openInterface(&(interfaces[i]), fd); - } -} - -static int processLineOfInput(Interface * interface) -{ - int ret = 1; - char *line = interface->buffer + interface->bufferPos; - - if (interface->cmd_list_OK >= 0) { - if (strcmp(line, INTERFACE_LIST_MODE_END) == 0) { - DEBUG("interface %i: process command " - "list\n", interface->num); - ret = processListOfCommands(interface->fd, - &(interface->permission), - &(interface->expired), - interface->cmd_list_OK, - interface->cmd_list); - DEBUG("interface %i: process command " - "list returned %i\n", interface->num, ret); - if (ret == 0) - commandSuccess(interface->fd); - else if (ret == COMMAND_RETURN_CLOSE - || interface->expired) - closeInterface(interface); - - printInterfaceOutBuffer(interface); - free_cmd_list(interface->cmd_list); - interface->cmd_list = NULL; - interface->cmd_list_OK = -1; - } else { - size_t len = strlen(line) + 1; - interface->cmd_list_size += len; - if (interface->cmd_list_size > - interface_max_command_list_size) { - ERROR("interface %i: command " - "list size (%lu) is " - "larger than the max " - "(%lu)\n", - interface->num, - (unsigned long)interface->cmd_list_size, - (unsigned long) - interface_max_command_list_size); - closeInterface(interface); - ret = COMMAND_RETURN_CLOSE; - } else - new_cmd_list_ptr(interface, line, len); - } - } else { - if (strcmp(line, INTERFACE_LIST_MODE_BEGIN) == 0) { - interface->cmd_list_OK = 0; - ret = 1; - } else if (strcmp(line, INTERFACE_LIST_OK_MODE_BEGIN) == 0) { - interface->cmd_list_OK = 1; - ret = 1; - } else { - DEBUG("interface %i: process command \"%s\"\n", - interface->num, line); - ret = processCommand(interface->fd, - &(interface->permission), line); - DEBUG("interface %i: command returned %i\n", - interface->num, ret); - if (ret == 0) - commandSuccess(interface->fd); - else if (ret == COMMAND_RETURN_CLOSE - || interface->expired) { - closeInterface(interface); - } - printInterfaceOutBuffer(interface); - } - } - - return ret; -} - -static int processBytesRead(Interface * interface, int bytesRead) -{ - int ret = 0; - char *buf_tail = &(interface->buffer[interface->bufferLength - 1]); - - while (bytesRead > 0) { - interface->bufferLength++; - bytesRead--; - buf_tail++; - if (*buf_tail == '\n') { - *buf_tail = '\0'; - if (interface->bufferLength > interface->bufferPos) { - if (*(buf_tail - 1) == '\r') - *(buf_tail - 1) = '\0'; - } - ret = processLineOfInput(interface); - if (interface->expired) - return ret; - interface->bufferPos = interface->bufferLength; - } - if (interface->bufferLength == INTERFACE_MAX_BUFFER_LENGTH) { - if (interface->bufferPos == 0) { - ERROR("interface %i: buffer overflow\n", - interface->num); - closeInterface(interface); - return 1; - } - if (interface->cmd_list_OK >= 0 && - interface->cmd_list && - !interface->cmd_list_dup) - cmd_list_clone(interface); - assert(interface->bufferLength >= interface->bufferPos - && "bufferLength >= bufferPos"); - interface->bufferLength -= interface->bufferPos; - memmove(interface->buffer, - interface->buffer + interface->bufferPos, - interface->bufferLength); - interface->bufferPos = 0; - } - if (ret == COMMAND_RETURN_KILL || ret == COMMAND_RETURN_CLOSE) { - return ret; - } - - } - - return ret; -} - -static int interfaceReadInput(Interface * interface) -{ - int bytesRead; - - bytesRead = read(interface->fd, - interface->buffer + interface->bufferLength, - INTERFACE_MAX_BUFFER_LENGTH - interface->bufferLength); - - if (bytesRead > 0) - return processBytesRead(interface, bytesRead); - else if (bytesRead == 0 || (bytesRead < 0 && errno != EINTR)) { - closeInterface(interface); - } else - return 0; - - return 1; -} - -static void addInterfacesReadyToReadAndListenSocketToFdSet(fd_set * fds, - int *fdmax) -{ - unsigned int i; - - FD_ZERO(fds); - addListenSocketsToFdSet(fds, fdmax); - - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0 && !interfaces[i].expired - && !interfaces[i].deferred_send) { - FD_SET(interfaces[i].fd, fds); - if (*fdmax < interfaces[i].fd) - *fdmax = interfaces[i].fd; - } - } -} - -static void addInterfacesForBufferFlushToFdSet(fd_set * fds, int *fdmax) -{ - unsigned int i; - - FD_ZERO(fds); - - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0 && !interfaces[i].expired - && interfaces[i].deferred_send) { - FD_SET(interfaces[i].fd, fds); - if (*fdmax < interfaces[i].fd) - *fdmax = interfaces[i].fd; - } - } -} - -static void closeNextErroredInterface(void) -{ - fd_set fds; - struct timeval tv; - unsigned int i; - - tv.tv_sec = 0; - tv.tv_usec = 0; - - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0) { - FD_ZERO(&fds); - FD_SET(interfaces[i].fd, &fds); - if (select(interfaces[i].fd + 1, - &fds, NULL, NULL, &tv) < 0) { - closeInterface(&interfaces[i]); - return; - } - } - } -} - -int doIOForInterfaces(void) -{ - fd_set rfds; - fd_set wfds; - fd_set efds; - unsigned int i; - int selret; - int fdmax; - - while (1) { - fdmax = 0; - - FD_ZERO( &efds ); - addInterfacesReadyToReadAndListenSocketToFdSet(&rfds, &fdmax); - addInterfacesForBufferFlushToFdSet(&wfds, &fdmax); - - registered_IO_add_fds(&fdmax, &rfds, &wfds, &efds); - - selret = select(fdmax + 1, &rfds, &wfds, &efds, NULL); - if (selret < 0 && errno == EINTR) - break; - - registered_IO_consume_fds(&selret, &rfds, &wfds, &efds); - - if (selret == 0) - break; - - if (selret < 0) { - closeNextErroredInterface(); - continue; - } - - getConnections(&rfds); - - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0 - && FD_ISSET(interfaces[i].fd, &rfds)) { - if (COMMAND_RETURN_KILL == - interfaceReadInput(&(interfaces[i]))) { - return COMMAND_RETURN_KILL; - } - interfaces[i].lastTime = time(NULL); - } - if (interfaces[i].fd >= 0 - && FD_ISSET(interfaces[i].fd, &wfds)) { - flushInterfaceBuffer(&interfaces[i]); - interfaces[i].lastTime = time(NULL); - } - } - - break; - } - - return 1; -} - -void initInterfaces(void) -{ - unsigned int i; - char *test; - ConfigParam *param; - - param = getConfigParam(CONF_CONN_TIMEOUT); - - if (param) { - interface_timeout = strtol(param->value, &test, 10); - if (*test != '\0' || interface_timeout <= 0) { - FATAL("connection timeout \"%s\" is not a positive " - "integer, line %i\n", CONF_CONN_TIMEOUT, - param->line); - } - } - - param = getConfigParam(CONF_MAX_CONN); - - if (param) { - interface_max_connections = strtol(param->value, &test, 10); - if (*test != '\0' || interface_max_connections <= 0) { - FATAL("max connections \"%s\" is not a positive integer" - ", line %i\n", param->value, param->line); - } - } else - interface_max_connections = INTERFACE_MAX_CONNECTIONS_DEFAULT; - - param = getConfigParam(CONF_MAX_COMMAND_LIST_SIZE); - - if (param) { - long tmp = strtol(param->value, &test, 10); - if (*test != '\0' || tmp <= 0) { - FATAL("max command list size \"%s\" is not a positive " - "integer, line %i\n", param->value, param->line); - } - interface_max_command_list_size = tmp * 1024; - } - - param = getConfigParam(CONF_MAX_OUTPUT_BUFFER_SIZE); - - if (param) { - long tmp = strtol(param->value, &test, 10); - if (*test != '\0' || tmp <= 0) { - FATAL("max output buffer size \"%s\" is not a positive " - "integer, line %i\n", param->value, param->line); - } - interface_max_output_buffer_size = tmp * 1024; - } - - interfaces = xmalloc(sizeof(Interface) * interface_max_connections); - - list_cache = xcalloc(interface_list_cache_size, sizeof(struct strnode)); - list_cache_head = &(list_cache[0]); - list_cache_tail = &(list_cache[interface_list_cache_size - 1]); - - for (i = 0; i < interface_max_connections; i++) { - interfaces[i].fd = -1; - interfaces[i].send_buf = NULL; - interfaces[i].send_buf_size = 0; - interfaces[i].send_buf_alloc = 0; - interfaces[i].num = i; - } -} - -static void closeAllInterfaces(void) -{ - unsigned int i; - - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0) - closeInterface(&(interfaces[i])); - if (interfaces[i].send_buf) - free(interfaces[i].send_buf); - } - free(list_cache); -} - -void freeAllInterfaces(void) -{ - closeAllInterfaces(); - - free(interfaces); - - interface_max_connections = 0; -} - -void closeOldInterfaces(void) -{ - unsigned int i; - - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0) { - if (interfaces[i].expired) { - DEBUG("interface %i: expired\n", i); - closeInterface(&(interfaces[i])); - } else if (time(NULL) - interfaces[i].lastTime > - interface_timeout) { - DEBUG("interface %i: timeout\n", i); - closeInterface(&(interfaces[i])); - } - } - } -} - -static void flushInterfaceBuffer(Interface * interface) -{ - struct sllnode *buf; - ssize_t ret = 0; - - buf = interface->deferred_send; - while (buf) { - ret = write(interface->fd, buf->data, buf->size); - if (ret < 0) - break; - else if ((size_t)ret < buf->size) { - assert(interface->deferred_bytes >= (size_t)ret); - interface->deferred_bytes -= ret; - buf->data = (char *)buf->data + ret; - buf->size -= ret; - } else { - struct sllnode *tmp = buf; - size_t decr = (buf->size + sizeof(struct sllnode)); - - assert(interface->deferred_bytes >= decr); - interface->deferred_bytes -= decr; - buf = buf->next; - free(tmp); - interface->deferred_send = buf; - } - interface->lastTime = time(NULL); - } - - if (!interface->deferred_send) { - DEBUG("interface %i: buffer empty %lu\n", interface->num, - (unsigned long)interface->deferred_bytes); - assert(interface->deferred_bytes == 0); - } else if (ret < 0 && errno != EAGAIN && errno != EINTR) { - /* cause interface to close */ - DEBUG("interface %i: problems flushing buffer\n", - interface->num); - buf = interface->deferred_send; - do { - struct sllnode *prev = buf; - buf = buf->next; - free(prev); - } while (buf); - interface->deferred_send = NULL; - interface->deferred_bytes = 0; - interface->expired = 1; - } -} - -int interfacePrintWithFD(int fd, const char *buffer, size_t buflen) -{ - static unsigned int i; - size_t copylen; - Interface *interface; - - assert(fd >= 0); - - if (i >= interface_max_connections || - interfaces[i].fd < 0 || interfaces[i].fd != fd) { - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd == fd) - break; - } - if (i == interface_max_connections) - return -1; - } - - /* if fd isn't found or interfaces is going to be closed, do nothing */ - if (interfaces[i].expired) - return 0; - - interface = interfaces + i; - - while (buflen > 0 && !interface->expired) { - size_t left; - - assert(interface->send_buf_size >= interface->send_buf_used); - left = interface->send_buf_size - interface->send_buf_used; - - copylen = buflen > left ? left : buflen; - memcpy(interface->send_buf + interface->send_buf_used, buffer, - copylen); - buflen -= copylen; - interface->send_buf_used += copylen; - buffer += copylen; - if (interface->send_buf_used >= interface->send_buf_size) - printInterfaceOutBuffer(interface); - } - - return 0; -} - -static void printInterfaceOutBuffer(Interface * interface) -{ - ssize_t ret; - struct sllnode *buf; - - if (interface->fd < 0 || interface->expired || - !interface->send_buf_used) - return; - - if ((buf = interface->deferred_send)) { - interface->deferred_bytes += sizeof(struct sllnode) - + interface->send_buf_used; - if (interface->deferred_bytes > - interface_max_output_buffer_size) { - ERROR("interface %i: output buffer size (%lu) is " - "larger than the max (%lu)\n", - interface->num, - (unsigned long)interface->deferred_bytes, - (unsigned long)interface_max_output_buffer_size); - /* cause interface to close */ - interface->expired = 1; - do { - struct sllnode *prev = buf; - buf = buf->next; - free(prev); - } while (buf); - interface->deferred_send = NULL; - interface->deferred_bytes = 0; - } else { - while (buf->next) - buf = buf->next; - buf->next = new_sllnode(interface->send_buf, - interface->send_buf_used); - } - } else { - if ((ret = write(interface->fd, interface->send_buf, - interface->send_buf_used)) < 0) { - if (errno == EAGAIN || errno == EINTR) { - interface->deferred_send = - new_sllnode(interface->send_buf, - interface->send_buf_used); - } else { - DEBUG("interface %i: problems writing\n", - interface->num); - interface->expired = 1; - return; - } - } else if ((size_t)ret < interface->send_buf_used) { - interface->deferred_send = - new_sllnode(interface->send_buf + ret, - interface->send_buf_used - ret); - } - if (interface->deferred_send) { - DEBUG("interface %i: buffer created\n", interface->num); - interface->deferred_bytes = - interface->deferred_send->size - + sizeof(struct sllnode); - } - } - - interface->send_buf_used = 0; -} - diff --git a/src/interface.h b/src/interface.h deleted file mode 100644 index c83381319..000000000 --- a/src/interface.h +++ /dev/null @@ -1,32 +0,0 @@ -/* the Music Player Daemon (MPD) - * Copyright (C) 2003-2007 by Warren Dukes (warren.dukes@gmail.com) - * This project's homepage is: http://www.musicpd.org - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - */ - -#ifndef INTERFACE_H -#define INTERFACE_H - -#include "os_compat.h" - -void initInterfaces(void); -void openAInterface(int fd, const struct sockaddr *addr); -void freeAllInterfaces(void); -void closeOldInterfaces(void); -int interfacePrintWithFD(int fd, const char *buffer, size_t len); - -int doIOForInterfaces(void); - -#endif diff --git a/src/listen.c b/src/listen.c index 2b9b38619..4adebb66a 100644 --- a/src/listen.c +++ b/src/listen.c @@ -17,7 +17,7 @@ */ #include "listen.h" -#include "interface.h" +#include "client.h" #include "conf.h" #include "log.h" #include "utils.h" diff --git a/src/main.c b/src/main.c index 9d75fadc7..45dc962ce 100644 --- a/src/main.c +++ b/src/main.c @@ -16,7 +16,7 @@ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ -#include "interface.h" +#include "client.h" #include "command.h" #include "playlist.h" #include "directory.h" diff --git a/src/myfprintf.c b/src/myfprintf.c index 7e4f4678d..abb2fdde3 100644 --- a/src/myfprintf.c +++ b/src/myfprintf.c @@ -17,7 +17,7 @@ */ #include "myfprintf.h" -#include "interface.h" +#include "client.h" #include "path.h" #include "utils.h" #include "os_compat.h" -- cgit v1.2.3 From e97c4e27d34c15324bf71d5dc8443f61a1e26a46 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:02:58 +0200 Subject: client: renamed Interface to struct client Second patch: rename the internal struct name. We will eventually export this type as an opaque forward-declared struct later, so we can pass a struct pointer instead of a file descriptor, which would save us an expensive linear lookup. --- src/client.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index a6ca85497..37d150775 100644 --- a/src/client.c +++ b/src/client.c @@ -59,7 +59,7 @@ static struct strnode *list_cache; static struct strnode *list_cache_head; static struct strnode *list_cache_tail; -typedef struct _Interface { +struct client { char buffer[INTERFACE_MAX_BUFFER_LENGTH]; size_t bufferLength; size_t bufferPos; @@ -81,16 +81,16 @@ typedef struct _Interface { size_t send_buf_used; /* bytes used this instance */ size_t send_buf_size; /* bytes usable this instance */ size_t send_buf_alloc; /* bytes actually allocated */ -} Interface; +}; -static Interface *interfaces; +static struct client *interfaces; -static void flushInterfaceBuffer(Interface * interface); +static void flushInterfaceBuffer(struct client *interface); -static void printInterfaceOutBuffer(Interface * interface); +static void printInterfaceOutBuffer(struct client *interface); #ifdef SO_SNDBUF -static size_t get_default_snd_buf_size(Interface * interface) +static size_t get_default_snd_buf_size(struct client *interface) { int new_size; socklen_t sockOptLen = sizeof(int); @@ -106,13 +106,13 @@ static size_t get_default_snd_buf_size(Interface * interface) return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; } #else /* !SO_SNDBUF */ -static size_t get_default_snd_buf_size(Interface * interface) +static size_t get_default_snd_buf_size(struct client *interface) { return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; } #endif /* !SO_SNDBUF */ -static void set_send_buf_size(Interface * interface) +static void set_send_buf_size(struct client *interface) { size_t new_size = get_default_snd_buf_size(interface); if (interface->send_buf_size != new_size) { @@ -127,7 +127,7 @@ static void set_send_buf_size(Interface * interface) } } -static void openInterface(Interface * interface, int fd) +static void openInterface(struct client *interface, int fd) { assert(interface->fd < 0); @@ -168,7 +168,7 @@ static void free_cmd_list(struct strnode *list) } } -static void cmd_list_clone(Interface * interface) +static void cmd_list_clone(struct client *interface) { struct strnode *new = dup_strlist(interface->cmd_list); free_cmd_list(interface->cmd_list); @@ -181,7 +181,7 @@ static void cmd_list_clone(Interface * interface) interface->cmd_list_tail = new; } -static void new_cmd_list_ptr(Interface * interface, char *s, const int size) +static void new_cmd_list_ptr(struct client *interface, char *s, const int size) { int i; struct strnode *new; @@ -209,7 +209,7 @@ out: interface->cmd_list = interface->cmd_list_tail = new; } -static void closeInterface(Interface * interface) +static void closeInterface(struct client *interface) { struct sllnode *buf; if (interface->fd < 0) @@ -284,7 +284,7 @@ void openAInterface(int fd, const struct sockaddr *addr) } } -static int processLineOfInput(Interface * interface) +static int processLineOfInput(struct client *interface) { int ret = 1; char *line = interface->buffer + interface->bufferPos; @@ -355,7 +355,7 @@ static int processLineOfInput(Interface * interface) return ret; } -static int processBytesRead(Interface * interface, int bytesRead) +static int processBytesRead(struct client *interface, int bytesRead) { int ret = 0; char *buf_tail = &(interface->buffer[interface->bufferLength - 1]); @@ -403,7 +403,7 @@ static int processBytesRead(Interface * interface, int bytesRead) return ret; } -static int interfaceReadInput(Interface * interface) +static int interfaceReadInput(struct client *interface) { int bytesRead; @@ -583,7 +583,7 @@ void initInterfaces(void) interface_max_output_buffer_size = tmp * 1024; } - interfaces = xmalloc(sizeof(Interface) * interface_max_connections); + interfaces = xmalloc(sizeof(interfaces[0]) * interface_max_connections); list_cache = xcalloc(interface_list_cache_size, sizeof(struct strnode)); list_cache_head = &(list_cache[0]); @@ -638,7 +638,7 @@ void closeOldInterfaces(void) } } -static void flushInterfaceBuffer(Interface * interface) +static void flushInterfaceBuffer(struct client *interface) { struct sllnode *buf; ssize_t ret = 0; @@ -690,7 +690,7 @@ int interfacePrintWithFD(int fd, const char *buffer, size_t buflen) { static unsigned int i; size_t copylen; - Interface *interface; + struct client *interface; assert(fd >= 0); @@ -729,7 +729,7 @@ int interfacePrintWithFD(int fd, const char *buffer, size_t buflen) return 0; } -static void printInterfaceOutBuffer(Interface * interface) +static void printInterfaceOutBuffer(struct client *interface) { ssize_t ret; struct sllnode *buf; -- cgit v1.2.3 From ea2592efafb903bd8afb362daab8f3ae9b3a86c3 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:02:59 +0200 Subject: client: renamed internal functions and variables Rename all static functions, variables and macros which have "interface" in their name to something nicer prefixed with "client_". --- src/client.c | 593 +++++++++++++++++++++++++++++------------------------------ 1 file changed, 296 insertions(+), 297 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 37d150775..63595da03 100644 --- a/src/client.c +++ b/src/client.c @@ -33,34 +33,34 @@ #define GREETING "OK MPD " PROTOCOL_VERSION "\n" -#define INTERFACE_MAX_BUFFER_LENGTH (40960) -#define INTERFACE_LIST_MODE_BEGIN "command_list_begin" -#define INTERFACE_LIST_OK_MODE_BEGIN "command_list_ok_begin" -#define INTERFACE_LIST_MODE_END "command_list_end" -#define INTERFACE_DEFAULT_OUT_BUFFER_SIZE (4096) -#define INTERFACE_TIMEOUT_DEFAULT (60) -#define INTERFACE_MAX_CONNECTIONS_DEFAULT (10) -#define INTERFACE_MAX_COMMAND_LIST_DEFAULT (2048*1024) -#define INTERFACE_MAX_OUTPUT_BUFFER_SIZE_DEFAULT (8192*1024) - -/* set this to zero to indicate we have no possible interfaces */ -static unsigned int interface_max_connections; /*INTERFACE_MAX_CONNECTIONS_DEFAULT; */ -static int interface_timeout = INTERFACE_TIMEOUT_DEFAULT; -static size_t interface_max_command_list_size = - INTERFACE_MAX_COMMAND_LIST_DEFAULT; -static size_t interface_max_output_buffer_size = - INTERFACE_MAX_OUTPUT_BUFFER_SIZE_DEFAULT; +#define CLIENT_MAX_BUFFER_LENGTH (40960) +#define CLIENT_LIST_MODE_BEGIN "command_list_begin" +#define CLIENT_LIST_OK_MODE_BEGIN "command_list_ok_begin" +#define CLIENT_LIST_MODE_END "command_list_end" +#define CLIENT_DEFAULT_OUT_BUFFER_SIZE (4096) +#define CLIENT_TIMEOUT_DEFAULT (60) +#define CLIENT_MAX_CONNECTIONS_DEFAULT (10) +#define CLIENT_MAX_COMMAND_LIST_DEFAULT (2048*1024) +#define CLIENT_MAX_OUTPUT_BUFFER_SIZE_DEFAULT (8192*1024) + +/* set this to zero to indicate we have no possible clients */ +static unsigned int client_max_connections; /*CLIENT_MAX_CONNECTIONS_DEFAULT; */ +static int client_timeout = CLIENT_TIMEOUT_DEFAULT; +static size_t client_max_command_list_size = + CLIENT_MAX_COMMAND_LIST_DEFAULT; +static size_t client_max_output_buffer_size = + CLIENT_MAX_OUTPUT_BUFFER_SIZE_DEFAULT; /* maybe make conf option for this, or... 32 might be good enough */ -static long int interface_list_cache_size = 32; +static long int client_list_cache_size = 32; -/* shared globally between all interfaces: */ +/* shared globally between all clients: */ static struct strnode *list_cache; static struct strnode *list_cache_head; static struct strnode *list_cache_tail; struct client { - char buffer[INTERFACE_MAX_BUFFER_LENGTH]; + char buffer[CLIENT_MAX_BUFFER_LENGTH]; size_t bufferLength; size_t bufferPos; int fd; /* file descriptor */ @@ -73,9 +73,9 @@ struct client { int cmd_list_dup; /* has the cmd_list been copied to private space? */ struct sllnode *deferred_send; /* for output if client is slow */ size_t deferred_bytes; /* mem deferred_send consumes */ - int expired; /* set whether this interface should be closed on next - check of old interfaces */ - unsigned int num; /* interface number */ + int expired; /* set whether this client should be closed on next + check of old clients */ + unsigned int num; /* client number */ char *send_buf; size_t send_buf_used; /* bytes used this instance */ @@ -83,71 +83,71 @@ struct client { size_t send_buf_alloc; /* bytes actually allocated */ }; -static struct client *interfaces; +static struct client *clients; -static void flushInterfaceBuffer(struct client *interface); +static void client_write_deferred(struct client *client); -static void printInterfaceOutBuffer(struct client *interface); +static void client_write_output(struct client *client); #ifdef SO_SNDBUF -static size_t get_default_snd_buf_size(struct client *interface) +static size_t get_default_snd_buf_size(struct client *client) { int new_size; socklen_t sockOptLen = sizeof(int); - if (getsockopt(interface->fd, SOL_SOCKET, SO_SNDBUF, + if (getsockopt(client->fd, SOL_SOCKET, SO_SNDBUF, (char *)&new_size, &sockOptLen) < 0) { DEBUG("problem getting sockets send buffer size\n"); - return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; + return CLIENT_DEFAULT_OUT_BUFFER_SIZE; } if (new_size > 0) return (size_t)new_size; DEBUG("sockets send buffer size is not positive\n"); - return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; + return CLIENT_DEFAULT_OUT_BUFFER_SIZE; } #else /* !SO_SNDBUF */ -static size_t get_default_snd_buf_size(struct client *interface) +static size_t get_default_snd_buf_size(struct client *client) { - return INTERFACE_DEFAULT_OUT_BUFFER_SIZE; + return CLIENT_DEFAULT_OUT_BUFFER_SIZE; } #endif /* !SO_SNDBUF */ -static void set_send_buf_size(struct client *interface) +static void set_send_buf_size(struct client *client) { - size_t new_size = get_default_snd_buf_size(interface); - if (interface->send_buf_size != new_size) { - interface->send_buf_size = new_size; + size_t new_size = get_default_snd_buf_size(client); + if (client->send_buf_size != new_size) { + client->send_buf_size = new_size; /* don't resize to get smaller, only bigger */ - if (interface->send_buf_alloc < new_size) { - if (interface->send_buf) - free(interface->send_buf); - interface->send_buf = xmalloc(new_size); - interface->send_buf_alloc = new_size; + if (client->send_buf_alloc < new_size) { + if (client->send_buf) + free(client->send_buf); + client->send_buf = xmalloc(new_size); + client->send_buf_alloc = new_size; } } } -static void openInterface(struct client *interface, int fd) +static void client_init(struct client *client, int fd) { - assert(interface->fd < 0); - - interface->cmd_list_size = 0; - interface->cmd_list_dup = 0; - interface->cmd_list_OK = -1; - interface->bufferLength = 0; - interface->bufferPos = 0; - interface->fd = fd; + assert(client->fd < 0); + + client->cmd_list_size = 0; + client->cmd_list_dup = 0; + client->cmd_list_OK = -1; + client->bufferLength = 0; + client->bufferPos = 0; + client->fd = fd; set_nonblocking(fd); - interface->lastTime = time(NULL); - interface->cmd_list = NULL; - interface->cmd_list_tail = NULL; - interface->deferred_send = NULL; - interface->expired = 0; - interface->deferred_bytes = 0; - interface->send_buf_used = 0; + client->lastTime = time(NULL); + client->cmd_list = NULL; + client->cmd_list_tail = NULL; + client->deferred_send = NULL; + client->expired = 0; + client->deferred_bytes = 0; + client->send_buf_used = 0; - interface->permission = getDefaultPermissions(); - set_send_buf_size(interface); + client->permission = getDefaultPermissions(); + set_send_buf_size(client); xwrite(fd, GREETING, strlen(GREETING)); } @@ -168,26 +168,26 @@ static void free_cmd_list(struct strnode *list) } } -static void cmd_list_clone(struct client *interface) +static void cmd_list_clone(struct client *client) { - struct strnode *new = dup_strlist(interface->cmd_list); - free_cmd_list(interface->cmd_list); - interface->cmd_list = new; - interface->cmd_list_dup = 1; + struct strnode *new = dup_strlist(client->cmd_list); + free_cmd_list(client->cmd_list); + client->cmd_list = new; + client->cmd_list_dup = 1; /* new tail */ while (new && new->next) new = new->next; - interface->cmd_list_tail = new; + client->cmd_list_tail = new; } -static void new_cmd_list_ptr(struct client *interface, char *s, const int size) +static void new_cmd_list_ptr(struct client *client, char *s, const int size) { int i; struct strnode *new; - if (!interface->cmd_list_dup) { - for (i = interface_list_cache_size - 1; i >= 0; --i) { + if (!client->cmd_list_dup) { + for (i = client_list_cache_size - 1; i >= 0; --i) { if (list_cache[i].data) continue; new = &(list_cache[i]); @@ -199,49 +199,49 @@ static void new_cmd_list_ptr(struct client *interface, char *s, const int size) } /* allocate from the heap */ - new = interface->cmd_list_dup ? new_strnode_dup(s, size) + new = client->cmd_list_dup ? new_strnode_dup(s, size) : new_strnode(s); out: - if (interface->cmd_list) { - interface->cmd_list_tail->next = new; - interface->cmd_list_tail = new; + if (client->cmd_list) { + client->cmd_list_tail->next = new; + client->cmd_list_tail = new; } else - interface->cmd_list = interface->cmd_list_tail = new; + client->cmd_list = client->cmd_list_tail = new; } -static void closeInterface(struct client *interface) +static void client_close(struct client *client) { struct sllnode *buf; - if (interface->fd < 0) + if (client->fd < 0) return; - xclose(interface->fd); - interface->fd = -1; + xclose(client->fd); + client->fd = -1; - if (interface->cmd_list) { - free_cmd_list(interface->cmd_list); - interface->cmd_list = NULL; + if (client->cmd_list) { + free_cmd_list(client->cmd_list); + client->cmd_list = NULL; } - if ((buf = interface->deferred_send)) { + if ((buf = client->deferred_send)) { do { struct sllnode *prev = buf; buf = buf->next; free(prev); } while (buf); - interface->deferred_send = NULL; + client->deferred_send = NULL; } - SECURE("interface %i: closed\n", interface->num); + SECURE("client %i: closed\n", client->num); } void openAInterface(int fd, const struct sockaddr *addr) { unsigned int i; - for (i = 0; i < interface_max_connections - && interfaces[i].fd >= 0; i++) /* nothing */ ; + for (i = 0; i < client_max_connections + && clients[i].fd >= 0; i++) /* nothing */ ; - if (i == interface_max_connections) { + if (i == client_max_connections) { ERROR("Max Connections Reached!\n"); xclose(fd); } else { @@ -279,120 +279,120 @@ void openAInterface(int fd, const struct sockaddr *addr) default: hostname = "unknown"; } - SECURE("interface %i: opened from %s\n", i, hostname); - openInterface(&(interfaces[i]), fd); + SECURE("client %i: opened from %s\n", i, hostname); + client_init(&(clients[i]), fd); } } -static int processLineOfInput(struct client *interface) +static int client_process_line(struct client *client) { int ret = 1; - char *line = interface->buffer + interface->bufferPos; - - if (interface->cmd_list_OK >= 0) { - if (strcmp(line, INTERFACE_LIST_MODE_END) == 0) { - DEBUG("interface %i: process command " - "list\n", interface->num); - ret = processListOfCommands(interface->fd, - &(interface->permission), - &(interface->expired), - interface->cmd_list_OK, - interface->cmd_list); - DEBUG("interface %i: process command " - "list returned %i\n", interface->num, ret); + char *line = client->buffer + client->bufferPos; + + if (client->cmd_list_OK >= 0) { + if (strcmp(line, CLIENT_LIST_MODE_END) == 0) { + DEBUG("client %i: process command " + "list\n", client->num); + ret = processListOfCommands(client->fd, + &(client->permission), + &(client->expired), + client->cmd_list_OK, + client->cmd_list); + DEBUG("client %i: process command " + "list returned %i\n", client->num, ret); if (ret == 0) - commandSuccess(interface->fd); + commandSuccess(client->fd); else if (ret == COMMAND_RETURN_CLOSE - || interface->expired) - closeInterface(interface); + || client->expired) + client_close(client); - printInterfaceOutBuffer(interface); - free_cmd_list(interface->cmd_list); - interface->cmd_list = NULL; - interface->cmd_list_OK = -1; + client_write_output(client); + free_cmd_list(client->cmd_list); + client->cmd_list = NULL; + client->cmd_list_OK = -1; } else { size_t len = strlen(line) + 1; - interface->cmd_list_size += len; - if (interface->cmd_list_size > - interface_max_command_list_size) { - ERROR("interface %i: command " + client->cmd_list_size += len; + if (client->cmd_list_size > + client_max_command_list_size) { + ERROR("client %i: command " "list size (%lu) is " "larger than the max " "(%lu)\n", - interface->num, - (unsigned long)interface->cmd_list_size, + client->num, + (unsigned long)client->cmd_list_size, (unsigned long) - interface_max_command_list_size); - closeInterface(interface); + client_max_command_list_size); + client_close(client); ret = COMMAND_RETURN_CLOSE; } else - new_cmd_list_ptr(interface, line, len); + new_cmd_list_ptr(client, line, len); } } else { - if (strcmp(line, INTERFACE_LIST_MODE_BEGIN) == 0) { - interface->cmd_list_OK = 0; + if (strcmp(line, CLIENT_LIST_MODE_BEGIN) == 0) { + client->cmd_list_OK = 0; ret = 1; - } else if (strcmp(line, INTERFACE_LIST_OK_MODE_BEGIN) == 0) { - interface->cmd_list_OK = 1; + } else if (strcmp(line, CLIENT_LIST_OK_MODE_BEGIN) == 0) { + client->cmd_list_OK = 1; ret = 1; } else { - DEBUG("interface %i: process command \"%s\"\n", - interface->num, line); - ret = processCommand(interface->fd, - &(interface->permission), line); - DEBUG("interface %i: command returned %i\n", - interface->num, ret); + DEBUG("client %i: process command \"%s\"\n", + client->num, line); + ret = processCommand(client->fd, + &(client->permission), line); + DEBUG("client %i: command returned %i\n", + client->num, ret); if (ret == 0) - commandSuccess(interface->fd); + commandSuccess(client->fd); else if (ret == COMMAND_RETURN_CLOSE - || interface->expired) { - closeInterface(interface); + || client->expired) { + client_close(client); } - printInterfaceOutBuffer(interface); + client_write_output(client); } } return ret; } -static int processBytesRead(struct client *interface, int bytesRead) +static int client_input_received(struct client *client, int bytesRead) { int ret = 0; - char *buf_tail = &(interface->buffer[interface->bufferLength - 1]); + char *buf_tail = &(client->buffer[client->bufferLength - 1]); while (bytesRead > 0) { - interface->bufferLength++; + client->bufferLength++; bytesRead--; buf_tail++; if (*buf_tail == '\n') { *buf_tail = '\0'; - if (interface->bufferLength > interface->bufferPos) { + if (client->bufferLength > client->bufferPos) { if (*(buf_tail - 1) == '\r') *(buf_tail - 1) = '\0'; } - ret = processLineOfInput(interface); - if (interface->expired) + ret = client_process_line(client); + if (client->expired) return ret; - interface->bufferPos = interface->bufferLength; + client->bufferPos = client->bufferLength; } - if (interface->bufferLength == INTERFACE_MAX_BUFFER_LENGTH) { - if (interface->bufferPos == 0) { - ERROR("interface %i: buffer overflow\n", - interface->num); - closeInterface(interface); + if (client->bufferLength == CLIENT_MAX_BUFFER_LENGTH) { + if (client->bufferPos == 0) { + ERROR("client %i: buffer overflow\n", + client->num); + client_close(client); return 1; } - if (interface->cmd_list_OK >= 0 && - interface->cmd_list && - !interface->cmd_list_dup) - cmd_list_clone(interface); - assert(interface->bufferLength >= interface->bufferPos + if (client->cmd_list_OK >= 0 && + client->cmd_list && + !client->cmd_list_dup) + cmd_list_clone(client); + assert(client->bufferLength >= client->bufferPos && "bufferLength >= bufferPos"); - interface->bufferLength -= interface->bufferPos; - memmove(interface->buffer, - interface->buffer + interface->bufferPos, - interface->bufferLength); - interface->bufferPos = 0; + client->bufferLength -= client->bufferPos; + memmove(client->buffer, + client->buffer + client->bufferPos, + client->bufferLength); + client->bufferPos = 0; } if (ret == COMMAND_RETURN_KILL || ret == COMMAND_RETURN_CLOSE) { return ret; @@ -403,54 +403,53 @@ static int processBytesRead(struct client *interface, int bytesRead) return ret; } -static int interfaceReadInput(struct client *interface) +static int client_read(struct client *client) { int bytesRead; - bytesRead = read(interface->fd, - interface->buffer + interface->bufferLength, - INTERFACE_MAX_BUFFER_LENGTH - interface->bufferLength); + bytesRead = read(client->fd, + client->buffer + client->bufferLength, + CLIENT_MAX_BUFFER_LENGTH - client->bufferLength); if (bytesRead > 0) - return processBytesRead(interface, bytesRead); + return client_input_received(client, bytesRead); else if (bytesRead == 0 || (bytesRead < 0 && errno != EINTR)) { - closeInterface(interface); + client_close(client); } else return 0; return 1; } -static void addInterfacesReadyToReadAndListenSocketToFdSet(fd_set * fds, - int *fdmax) +static void client_manager_register_read_fd(fd_set * fds, int *fdmax) { unsigned int i; FD_ZERO(fds); addListenSocketsToFdSet(fds, fdmax); - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0 && !interfaces[i].expired - && !interfaces[i].deferred_send) { - FD_SET(interfaces[i].fd, fds); - if (*fdmax < interfaces[i].fd) - *fdmax = interfaces[i].fd; + for (i = 0; i < client_max_connections; i++) { + if (clients[i].fd >= 0 && !clients[i].expired + && !clients[i].deferred_send) { + FD_SET(clients[i].fd, fds); + if (*fdmax < clients[i].fd) + *fdmax = clients[i].fd; } } } -static void addInterfacesForBufferFlushToFdSet(fd_set * fds, int *fdmax) +static void client_manager_register_write_fd(fd_set * fds, int *fdmax) { unsigned int i; FD_ZERO(fds); - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0 && !interfaces[i].expired - && interfaces[i].deferred_send) { - FD_SET(interfaces[i].fd, fds); - if (*fdmax < interfaces[i].fd) - *fdmax = interfaces[i].fd; + for (i = 0; i < client_max_connections; i++) { + if (clients[i].fd >= 0 && !clients[i].expired + && clients[i].deferred_send) { + FD_SET(clients[i].fd, fds); + if (*fdmax < clients[i].fd) + *fdmax = clients[i].fd; } } } @@ -464,13 +463,13 @@ static void closeNextErroredInterface(void) tv.tv_sec = 0; tv.tv_usec = 0; - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0) { + for (i = 0; i < client_max_connections; i++) { + if (clients[i].fd >= 0) { FD_ZERO(&fds); - FD_SET(interfaces[i].fd, &fds); - if (select(interfaces[i].fd + 1, + FD_SET(clients[i].fd, &fds); + if (select(clients[i].fd + 1, &fds, NULL, NULL, &tv) < 0) { - closeInterface(&interfaces[i]); + client_close(&clients[i]); return; } } @@ -490,8 +489,8 @@ int doIOForInterfaces(void) fdmax = 0; FD_ZERO( &efds ); - addInterfacesReadyToReadAndListenSocketToFdSet(&rfds, &fdmax); - addInterfacesForBufferFlushToFdSet(&wfds, &fdmax); + client_manager_register_read_fd(&rfds, &fdmax); + client_manager_register_write_fd(&wfds, &fdmax); registered_IO_add_fds(&fdmax, &rfds, &wfds, &efds); @@ -511,19 +510,19 @@ int doIOForInterfaces(void) getConnections(&rfds); - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0 - && FD_ISSET(interfaces[i].fd, &rfds)) { + for (i = 0; i < client_max_connections; i++) { + if (clients[i].fd >= 0 + && FD_ISSET(clients[i].fd, &rfds)) { if (COMMAND_RETURN_KILL == - interfaceReadInput(&(interfaces[i]))) { + client_read(&(clients[i]))) { return COMMAND_RETURN_KILL; } - interfaces[i].lastTime = time(NULL); + clients[i].lastTime = time(NULL); } - if (interfaces[i].fd >= 0 - && FD_ISSET(interfaces[i].fd, &wfds)) { - flushInterfaceBuffer(&interfaces[i]); - interfaces[i].lastTime = time(NULL); + if (clients[i].fd >= 0 + && FD_ISSET(clients[i].fd, &wfds)) { + client_write_deferred(&clients[i]); + clients[i].lastTime = time(NULL); } } @@ -542,8 +541,8 @@ void initInterfaces(void) param = getConfigParam(CONF_CONN_TIMEOUT); if (param) { - interface_timeout = strtol(param->value, &test, 10); - if (*test != '\0' || interface_timeout <= 0) { + client_timeout = strtol(param->value, &test, 10); + if (*test != '\0' || client_timeout <= 0) { FATAL("connection timeout \"%s\" is not a positive " "integer, line %i\n", CONF_CONN_TIMEOUT, param->line); @@ -553,13 +552,13 @@ void initInterfaces(void) param = getConfigParam(CONF_MAX_CONN); if (param) { - interface_max_connections = strtol(param->value, &test, 10); - if (*test != '\0' || interface_max_connections <= 0) { + client_max_connections = strtol(param->value, &test, 10); + if (*test != '\0' || client_max_connections <= 0) { FATAL("max connections \"%s\" is not a positive integer" ", line %i\n", param->value, param->line); } } else - interface_max_connections = INTERFACE_MAX_CONNECTIONS_DEFAULT; + client_max_connections = CLIENT_MAX_CONNECTIONS_DEFAULT; param = getConfigParam(CONF_MAX_COMMAND_LIST_SIZE); @@ -569,7 +568,7 @@ void initInterfaces(void) FATAL("max command list size \"%s\" is not a positive " "integer, line %i\n", param->value, param->line); } - interface_max_command_list_size = tmp * 1024; + client_max_command_list_size = tmp * 1024; } param = getConfigParam(CONF_MAX_OUTPUT_BUFFER_SIZE); @@ -580,109 +579,109 @@ void initInterfaces(void) FATAL("max output buffer size \"%s\" is not a positive " "integer, line %i\n", param->value, param->line); } - interface_max_output_buffer_size = tmp * 1024; + client_max_output_buffer_size = tmp * 1024; } - interfaces = xmalloc(sizeof(interfaces[0]) * interface_max_connections); + clients = xmalloc(sizeof(clients[0]) * client_max_connections); - list_cache = xcalloc(interface_list_cache_size, sizeof(struct strnode)); + list_cache = xcalloc(client_list_cache_size, sizeof(struct strnode)); list_cache_head = &(list_cache[0]); - list_cache_tail = &(list_cache[interface_list_cache_size - 1]); - - for (i = 0; i < interface_max_connections; i++) { - interfaces[i].fd = -1; - interfaces[i].send_buf = NULL; - interfaces[i].send_buf_size = 0; - interfaces[i].send_buf_alloc = 0; - interfaces[i].num = i; + list_cache_tail = &(list_cache[client_list_cache_size - 1]); + + for (i = 0; i < client_max_connections; i++) { + clients[i].fd = -1; + clients[i].send_buf = NULL; + clients[i].send_buf_size = 0; + clients[i].send_buf_alloc = 0; + clients[i].num = i; } } -static void closeAllInterfaces(void) +static void client_close_all(void) { unsigned int i; - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0) - closeInterface(&(interfaces[i])); - if (interfaces[i].send_buf) - free(interfaces[i].send_buf); + for (i = 0; i < client_max_connections; i++) { + if (clients[i].fd >= 0) + client_close(&(clients[i])); + if (clients[i].send_buf) + free(clients[i].send_buf); } free(list_cache); } void freeAllInterfaces(void) { - closeAllInterfaces(); + client_close_all(); - free(interfaces); + free(clients); - interface_max_connections = 0; + client_max_connections = 0; } void closeOldInterfaces(void) { unsigned int i; - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd >= 0) { - if (interfaces[i].expired) { - DEBUG("interface %i: expired\n", i); - closeInterface(&(interfaces[i])); - } else if (time(NULL) - interfaces[i].lastTime > - interface_timeout) { - DEBUG("interface %i: timeout\n", i); - closeInterface(&(interfaces[i])); + for (i = 0; i < client_max_connections; i++) { + if (clients[i].fd >= 0) { + if (clients[i].expired) { + DEBUG("client %i: expired\n", i); + client_close(&(clients[i])); + } else if (time(NULL) - clients[i].lastTime > + client_timeout) { + DEBUG("client %i: timeout\n", i); + client_close(&(clients[i])); } } } } -static void flushInterfaceBuffer(struct client *interface) +static void client_write_deferred(struct client *client) { struct sllnode *buf; ssize_t ret = 0; - buf = interface->deferred_send; + buf = client->deferred_send; while (buf) { - ret = write(interface->fd, buf->data, buf->size); + ret = write(client->fd, buf->data, buf->size); if (ret < 0) break; else if ((size_t)ret < buf->size) { - assert(interface->deferred_bytes >= (size_t)ret); - interface->deferred_bytes -= ret; + assert(client->deferred_bytes >= (size_t)ret); + client->deferred_bytes -= ret; buf->data = (char *)buf->data + ret; buf->size -= ret; } else { struct sllnode *tmp = buf; size_t decr = (buf->size + sizeof(struct sllnode)); - assert(interface->deferred_bytes >= decr); - interface->deferred_bytes -= decr; + assert(client->deferred_bytes >= decr); + client->deferred_bytes -= decr; buf = buf->next; free(tmp); - interface->deferred_send = buf; + client->deferred_send = buf; } - interface->lastTime = time(NULL); + client->lastTime = time(NULL); } - if (!interface->deferred_send) { - DEBUG("interface %i: buffer empty %lu\n", interface->num, - (unsigned long)interface->deferred_bytes); - assert(interface->deferred_bytes == 0); + if (!client->deferred_send) { + DEBUG("client %i: buffer empty %lu\n", client->num, + (unsigned long)client->deferred_bytes); + assert(client->deferred_bytes == 0); } else if (ret < 0 && errno != EAGAIN && errno != EINTR) { - /* cause interface to close */ - DEBUG("interface %i: problems flushing buffer\n", - interface->num); - buf = interface->deferred_send; + /* cause client to close */ + DEBUG("client %i: problems flushing buffer\n", + client->num); + buf = client->deferred_send; do { struct sllnode *prev = buf; buf = buf->next; free(prev); } while (buf); - interface->deferred_send = NULL; - interface->deferred_bytes = 0; - interface->expired = 1; + client->deferred_send = NULL; + client->deferred_bytes = 0; + client->expired = 1; } } @@ -690,105 +689,105 @@ int interfacePrintWithFD(int fd, const char *buffer, size_t buflen) { static unsigned int i; size_t copylen; - struct client *interface; + struct client *client; assert(fd >= 0); - if (i >= interface_max_connections || - interfaces[i].fd < 0 || interfaces[i].fd != fd) { - for (i = 0; i < interface_max_connections; i++) { - if (interfaces[i].fd == fd) + if (i >= client_max_connections || + clients[i].fd < 0 || clients[i].fd != fd) { + for (i = 0; i < client_max_connections; i++) { + if (clients[i].fd == fd) break; } - if (i == interface_max_connections) + if (i == client_max_connections) return -1; } - /* if fd isn't found or interfaces is going to be closed, do nothing */ - if (interfaces[i].expired) + /* if fd isn't found or client is going to be closed, do nothing */ + if (clients[i].expired) return 0; - interface = interfaces + i; + client = clients + i; - while (buflen > 0 && !interface->expired) { + while (buflen > 0 && !client->expired) { size_t left; - assert(interface->send_buf_size >= interface->send_buf_used); - left = interface->send_buf_size - interface->send_buf_used; + assert(client->send_buf_size >= client->send_buf_used); + left = client->send_buf_size - client->send_buf_used; copylen = buflen > left ? left : buflen; - memcpy(interface->send_buf + interface->send_buf_used, buffer, + memcpy(client->send_buf + client->send_buf_used, buffer, copylen); buflen -= copylen; - interface->send_buf_used += copylen; + client->send_buf_used += copylen; buffer += copylen; - if (interface->send_buf_used >= interface->send_buf_size) - printInterfaceOutBuffer(interface); + if (client->send_buf_used >= client->send_buf_size) + client_write_output(client); } return 0; } -static void printInterfaceOutBuffer(struct client *interface) +static void client_write_output(struct client *client) { ssize_t ret; struct sllnode *buf; - if (interface->fd < 0 || interface->expired || - !interface->send_buf_used) + if (client->fd < 0 || client->expired || + !client->send_buf_used) return; - if ((buf = interface->deferred_send)) { - interface->deferred_bytes += sizeof(struct sllnode) - + interface->send_buf_used; - if (interface->deferred_bytes > - interface_max_output_buffer_size) { - ERROR("interface %i: output buffer size (%lu) is " + if ((buf = client->deferred_send)) { + client->deferred_bytes += sizeof(struct sllnode) + + client->send_buf_used; + if (client->deferred_bytes > + client_max_output_buffer_size) { + ERROR("client %i: output buffer size (%lu) is " "larger than the max (%lu)\n", - interface->num, - (unsigned long)interface->deferred_bytes, - (unsigned long)interface_max_output_buffer_size); - /* cause interface to close */ - interface->expired = 1; + client->num, + (unsigned long)client->deferred_bytes, + (unsigned long)client_max_output_buffer_size); + /* cause client to close */ + client->expired = 1; do { struct sllnode *prev = buf; buf = buf->next; free(prev); } while (buf); - interface->deferred_send = NULL; - interface->deferred_bytes = 0; + client->deferred_send = NULL; + client->deferred_bytes = 0; } else { while (buf->next) buf = buf->next; - buf->next = new_sllnode(interface->send_buf, - interface->send_buf_used); + buf->next = new_sllnode(client->send_buf, + client->send_buf_used); } } else { - if ((ret = write(interface->fd, interface->send_buf, - interface->send_buf_used)) < 0) { + if ((ret = write(client->fd, client->send_buf, + client->send_buf_used)) < 0) { if (errno == EAGAIN || errno == EINTR) { - interface->deferred_send = - new_sllnode(interface->send_buf, - interface->send_buf_used); + client->deferred_send = + new_sllnode(client->send_buf, + client->send_buf_used); } else { - DEBUG("interface %i: problems writing\n", - interface->num); - interface->expired = 1; + DEBUG("client %i: problems writing\n", + client->num); + client->expired = 1; return; } - } else if ((size_t)ret < interface->send_buf_used) { - interface->deferred_send = - new_sllnode(interface->send_buf + ret, - interface->send_buf_used - ret); + } else if ((size_t)ret < client->send_buf_used) { + client->deferred_send = + new_sllnode(client->send_buf + ret, + client->send_buf_used - ret); } - if (interface->deferred_send) { - DEBUG("interface %i: buffer created\n", interface->num); - interface->deferred_bytes = - interface->deferred_send->size + if (client->deferred_send) { + DEBUG("client %i: buffer created\n", client->num); + client->deferred_bytes = + client->deferred_send->size + sizeof(struct sllnode); } } - interface->send_buf_used = 0; + client->send_buf_used = 0; } -- cgit v1.2.3 From 9cf1bf0671950024a21918335ab82ce9ef24cbf7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:03:02 +0200 Subject: client: renamed all public functions Functions which operate on the whole client list are prefixed with "client_manager_", and functions which handle just one client just get "client_". --- src/client.c | 12 ++++++------ src/client.h | 12 ++++++------ src/directory.c | 2 +- src/listen.c | 2 +- src/main.c | 8 ++++---- src/myfprintf.c | 2 +- 6 files changed, 19 insertions(+), 19 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 63595da03..87088e76f 100644 --- a/src/client.c +++ b/src/client.c @@ -234,7 +234,7 @@ static void client_close(struct client *client) SECURE("client %i: closed\n", client->num); } -void openAInterface(int fd, const struct sockaddr *addr) +void client_new(int fd, const struct sockaddr *addr) { unsigned int i; @@ -476,7 +476,7 @@ static void closeNextErroredInterface(void) } } -int doIOForInterfaces(void) +int client_manager_io(void) { fd_set rfds; fd_set wfds; @@ -532,7 +532,7 @@ int doIOForInterfaces(void) return 1; } -void initInterfaces(void) +void client_manager_init(void) { unsigned int i; char *test; @@ -610,7 +610,7 @@ static void client_close_all(void) free(list_cache); } -void freeAllInterfaces(void) +void client_manager_deinit(void) { client_close_all(); @@ -619,7 +619,7 @@ void freeAllInterfaces(void) client_max_connections = 0; } -void closeOldInterfaces(void) +void client_manager_expire(void) { unsigned int i; @@ -685,7 +685,7 @@ static void client_write_deferred(struct client *client) } } -int interfacePrintWithFD(int fd, const char *buffer, size_t buflen) +int client_print(int fd, const char *buffer, size_t buflen) { static unsigned int i; size_t copylen; diff --git a/src/client.h b/src/client.h index c83381319..64639b2af 100644 --- a/src/client.h +++ b/src/client.h @@ -21,12 +21,12 @@ #include "os_compat.h" -void initInterfaces(void); -void openAInterface(int fd, const struct sockaddr *addr); -void freeAllInterfaces(void); -void closeOldInterfaces(void); -int interfacePrintWithFD(int fd, const char *buffer, size_t len); +void client_manager_init(void); +void client_new(int fd, const struct sockaddr *addr); +void client_manager_deinit(void); +void client_manager_expire(void); +int client_print(int fd, const char *buffer, size_t len); -int doIOForInterfaces(void); +int client_manager_io(void); #endif diff --git a/src/directory.c b/src/directory.c index 6f3409356..46d309c76 100644 --- a/src/directory.c +++ b/src/directory.c @@ -177,7 +177,7 @@ int updateInit(int fd, List * pathList) finishSigHandlers(); closeAllListenSockets(); - freeAllInterfaces(); + client_manager_deinit(); finishPlaylist(); finishVolume(); diff --git a/src/listen.c b/src/listen.c index 4adebb66a..c40035279 100644 --- a/src/listen.c +++ b/src/listen.c @@ -297,7 +297,7 @@ void getConnections(fd_set * fds) if (FD_ISSET(listenSockets[i], fds)) { if ((fd = accept(listenSockets[i], &sockAddr, &socklen)) >= 0) { - openAInterface(fd, &sockAddr); + client_new(fd, &sockAddr); } else if (fd < 0 && (errno != EAGAIN && errno != EINTR)) { ERROR("Problems accept()'ing\n"); diff --git a/src/main.c b/src/main.c index 45dc962ce..363359831 100644 --- a/src/main.c +++ b/src/main.c @@ -415,7 +415,7 @@ int main(int argc, char *argv[]) initAudioConfig(); initAudioDriver(); initVolume(); - initInterfaces(); + client_manager_init(); initReplayGainState(); initNormalization(); initInputStream(); @@ -435,17 +435,17 @@ int main(int argc, char *argv[]) decoder_init(); read_state_file(); - while (COMMAND_RETURN_KILL != doIOForInterfaces() && + while (COMMAND_RETURN_KILL != client_manager_io() && COMMAND_RETURN_KILL != handlePendingSignals()) { syncPlayerAndPlaylist(); - closeOldInterfaces(); + client_manager_expire(); readDirectoryDBIfUpdateIsFinished(); } write_state_file(); ob_trigger_action(OB_ACTION_PAUSE_SET); finishZeroconf(); - freeAllInterfaces(); + client_manager_deinit(); closeAllListenSockets(); finishPlaylist(); diff --git a/src/myfprintf.c b/src/myfprintf.c index abb2fdde3..200c80334 100644 --- a/src/myfprintf.c +++ b/src/myfprintf.c @@ -49,7 +49,7 @@ void vfdprintf(const int fd, const char *fmt, va_list args) len = strlen(buf); if (fd == STDERR_FILENO || fd == STDOUT_FILENO || - interfacePrintWithFD(fd, buf, len) < 0) + client_print(fd, buf, len) < 0) blockingWrite(fd, buf, len); } -- cgit v1.2.3 From 1e6af706599c733a707e9fd32f0ef733d1908362 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:03:03 +0200 Subject: client: return early in client_new() This saves one level of indent. --- src/client.c | 61 ++++++++++++++++++++++++++++++------------------------------ 1 file changed, 31 insertions(+), 30 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 87088e76f..54c62bc83 100644 --- a/src/client.c +++ b/src/client.c @@ -237,6 +237,7 @@ static void client_close(struct client *client) void client_new(int fd, const struct sockaddr *addr) { unsigned int i; + const char *hostname; for (i = 0; i < client_max_connections && clients[i].fd >= 0; i++) /* nothing */ ; @@ -244,44 +245,44 @@ void client_new(int fd, const struct sockaddr *addr) if (i == client_max_connections) { ERROR("Max Connections Reached!\n"); xclose(fd); - } else { - const char *hostname; - switch (addr->sa_family) { + return; + } + + switch (addr->sa_family) { #ifdef HAVE_TCP - case AF_INET: - hostname = (const char *)inet_ntoa(((const struct sockaddr_in *) - addr)->sin_addr); - if (!hostname) - hostname = "error getting ipv4 address"; - break; + case AF_INET: + hostname = (const char *)inet_ntoa(((const struct sockaddr_in *) + addr)->sin_addr); + if (!hostname) + hostname = "error getting ipv4 address"; + break; #ifdef HAVE_IPV6 - case AF_INET6: - { - static char host[INET6_ADDRSTRLEN + 1]; - memset(host, 0, INET6_ADDRSTRLEN + 1); - if (inet_ntop(AF_INET6, (const void *) - &(((const struct sockaddr_in6 *)addr)-> - sin6_addr), host, - INET6_ADDRSTRLEN)) { - hostname = (const char *)host; - } else { - hostname = "error getting ipv6 address"; - } + case AF_INET6: + { + static char host[INET6_ADDRSTRLEN + 1]; + memset(host, 0, INET6_ADDRSTRLEN + 1); + if (inet_ntop(AF_INET6, (const void *) + &(((const struct sockaddr_in6 *)addr)-> + sin6_addr), host, + INET6_ADDRSTRLEN)) { + hostname = (const char *)host; + } else { + hostname = "error getting ipv6 address"; } - break; + } + break; #endif #endif /* HAVE_TCP */ #ifdef HAVE_UN - case AF_UNIX: - hostname = "local connection"; - break; + case AF_UNIX: + hostname = "local connection"; + break; #endif /* HAVE_UN */ - default: - hostname = "unknown"; - } - SECURE("client %i: opened from %s\n", i, hostname); - client_init(&(clients[i]), fd); + default: + hostname = "unknown"; } + SECURE("client %i: opened from %s\n", i, hostname); + client_init(&(clients[i]), fd); } static int client_process_line(struct client *client) -- cgit v1.2.3 From 7c5a476927734aec99ba87d744ac46ff7e890dd5 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:03:06 +0200 Subject: client: added function client_by_fd() The code becomes less complex and more readable when we move this linear search into a separate mini function. --- src/client.c | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 54c62bc83..c4693193f 100644 --- a/src/client.c +++ b/src/client.c @@ -686,30 +686,38 @@ static void client_write_deferred(struct client *client) } } -int client_print(int fd, const char *buffer, size_t buflen) +static struct client *client_by_fd(int fd) { static unsigned int i; + + assert(fd >= 0); + + if (i < client_max_connections && clients[i].fd >= 0 && + clients[i].fd == fd) + return &clients[i]; + + for (i = 0; i < client_max_connections; i++) + if (clients[i].fd == fd) + return &clients[i]; + + return NULL; +} + +int client_print(int fd, const char *buffer, size_t buflen) +{ size_t copylen; struct client *client; assert(fd >= 0); - if (i >= client_max_connections || - clients[i].fd < 0 || clients[i].fd != fd) { - for (i = 0; i < client_max_connections; i++) { - if (clients[i].fd == fd) - break; - } - if (i == client_max_connections) - return -1; - } + client = client_by_fd(fd); + if (client == NULL) + return -1; /* if fd isn't found or client is going to be closed, do nothing */ - if (clients[i].expired) + if (client->expired) return 0; - client = clients + i; - while (buflen > 0 && !client->expired) { size_t left; -- cgit v1.2.3 From a396943470949bf67065c6a3e3a98540ef4e966b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:03:48 +0200 Subject: client: allocate clients dynamically Due to the large buffers in the client struct, the static client array eats several megabytes of RAM with a maximum of only 10 clients. Stop this waste and allocate each client struct from the heap. --- src/client.c | 166 +++++++++++++++++++++++++++-------------------------------- 1 file changed, 76 insertions(+), 90 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index c4693193f..2dbc9d0fd 100644 --- a/src/client.c +++ b/src/client.c @@ -28,6 +28,7 @@ #include "myfprintf.h" #include "os_compat.h" #include "main_notify.h" +#include "dlist.h" #include "../config.h" @@ -60,6 +61,8 @@ static struct strnode *list_cache_head; static struct strnode *list_cache_tail; struct client { + struct list_head siblings; + char buffer[CLIENT_MAX_BUFFER_LENGTH]; size_t bufferLength; size_t bufferPos; @@ -83,7 +86,8 @@ struct client { size_t send_buf_alloc; /* bytes actually allocated */ }; -static struct client *clients; +static LIST_HEAD(clients); +static unsigned num_clients; static void client_write_deferred(struct client *client); @@ -129,7 +133,7 @@ static void set_send_buf_size(struct client *client) static void client_init(struct client *client, int fd) { - assert(client->fd < 0); + static unsigned int next_client_num; client->cmd_list_size = 0; client->cmd_list_dup = 0; @@ -144,6 +148,7 @@ static void client_init(struct client *client, int fd) client->deferred_send = NULL; client->expired = 0; client->deferred_bytes = 0; + client->num = next_client_num++; client->send_buf_used = 0; client->permission = getDefaultPermissions(); @@ -212,10 +217,15 @@ out: static void client_close(struct client *client) { struct sllnode *buf; - if (client->fd < 0) - return; + + assert(client->fd >= 0); + xclose(client->fd); - client->fd = -1; + + assert(num_clients > 0); + assert(!list_empty(&clients)); + list_del(&client->siblings); + --num_clients; if (client->cmd_list) { free_cmd_list(client->cmd_list); @@ -231,18 +241,19 @@ static void client_close(struct client *client) client->deferred_send = NULL; } + if (client->send_buf) + free(client->send_buf); + SECURE("client %i: closed\n", client->num); + free(client); } void client_new(int fd, const struct sockaddr *addr) { - unsigned int i; const char *hostname; + struct client *client; - for (i = 0; i < client_max_connections - && clients[i].fd >= 0; i++) /* nothing */ ; - - if (i == client_max_connections) { + if (num_clients >= client_max_connections) { ERROR("Max Connections Reached!\n"); xclose(fd); return; @@ -281,8 +292,12 @@ void client_new(int fd, const struct sockaddr *addr) default: hostname = "unknown"; } - SECURE("client %i: opened from %s\n", i, hostname); - client_init(&(clients[i]), fd); + + client = xcalloc(1, sizeof(*client)); + list_add(&client->siblings, &clients); + ++num_clients; + client_init(client, fd); + SECURE("client %i: opened from %s\n", client->num, hostname); } static int client_process_line(struct client *client) @@ -424,55 +439,52 @@ static int client_read(struct client *client) static void client_manager_register_read_fd(fd_set * fds, int *fdmax) { - unsigned int i; + struct client *client; FD_ZERO(fds); addListenSocketsToFdSet(fds, fdmax); - for (i = 0; i < client_max_connections; i++) { - if (clients[i].fd >= 0 && !clients[i].expired - && !clients[i].deferred_send) { - FD_SET(clients[i].fd, fds); - if (*fdmax < clients[i].fd) - *fdmax = clients[i].fd; + list_for_each_entry(client, &clients, siblings) { + if (!client->expired && !client->deferred_send) { + FD_SET(client->fd, fds); + if (*fdmax < client->fd) + *fdmax = client->fd; } } } static void client_manager_register_write_fd(fd_set * fds, int *fdmax) { - unsigned int i; + struct client *client; FD_ZERO(fds); - for (i = 0; i < client_max_connections; i++) { - if (clients[i].fd >= 0 && !clients[i].expired - && clients[i].deferred_send) { - FD_SET(clients[i].fd, fds); - if (*fdmax < clients[i].fd) - *fdmax = clients[i].fd; + list_for_each_entry(client, &clients, siblings) { + if (client->fd >= 0 && !client->expired + && client->deferred_send) { + FD_SET(client->fd, fds); + if (*fdmax < client->fd) + *fdmax = client->fd; } } } static void closeNextErroredInterface(void) { + struct client *client, *n; fd_set fds; struct timeval tv; - unsigned int i; tv.tv_sec = 0; tv.tv_usec = 0; - for (i = 0; i < client_max_connections; i++) { - if (clients[i].fd >= 0) { - FD_ZERO(&fds); - FD_SET(clients[i].fd, &fds); - if (select(clients[i].fd + 1, - &fds, NULL, NULL, &tv) < 0) { - client_close(&clients[i]); - return; - } + list_for_each_entry_safe(client, n, &clients, siblings) { + FD_ZERO(&fds); + FD_SET(client->fd, &fds); + if (select(client->fd + 1, + &fds, NULL, NULL, &tv) < 0) { + client_close(client); + return; } } } @@ -482,7 +494,7 @@ int client_manager_io(void) fd_set rfds; fd_set wfds; fd_set efds; - unsigned int i; + struct client *client, *n; int selret; int fdmax; @@ -511,19 +523,17 @@ int client_manager_io(void) getConnections(&rfds); - for (i = 0; i < client_max_connections; i++) { - if (clients[i].fd >= 0 - && FD_ISSET(clients[i].fd, &rfds)) { + list_for_each_entry_safe(client, n, &clients, siblings) { + if (FD_ISSET(client->fd, &rfds)) { if (COMMAND_RETURN_KILL == - client_read(&(clients[i]))) { + client_read(client)) { return COMMAND_RETURN_KILL; } - clients[i].lastTime = time(NULL); + client->lastTime = time(NULL); } - if (clients[i].fd >= 0 - && FD_ISSET(clients[i].fd, &wfds)) { - client_write_deferred(&clients[i]); - clients[i].lastTime = time(NULL); + if (FD_ISSET(client->fd, &wfds)) { + client_write_deferred(client); + client->lastTime = time(NULL); } } @@ -535,7 +545,6 @@ int client_manager_io(void) void client_manager_init(void) { - unsigned int i; char *test; ConfigParam *param; @@ -583,31 +592,19 @@ void client_manager_init(void) client_max_output_buffer_size = tmp * 1024; } - clients = xmalloc(sizeof(clients[0]) * client_max_connections); - list_cache = xcalloc(client_list_cache_size, sizeof(struct strnode)); list_cache_head = &(list_cache[0]); list_cache_tail = &(list_cache[client_list_cache_size - 1]); - - for (i = 0; i < client_max_connections; i++) { - clients[i].fd = -1; - clients[i].send_buf = NULL; - clients[i].send_buf_size = 0; - clients[i].send_buf_alloc = 0; - clients[i].num = i; - } } static void client_close_all(void) { - unsigned int i; + struct client *client, *n; + + list_for_each_entry_safe(client, n, &clients, siblings) + client_close(client); + num_clients = 0; - for (i = 0; i < client_max_connections; i++) { - if (clients[i].fd >= 0) - client_close(&(clients[i])); - if (clients[i].send_buf) - free(clients[i].send_buf); - } free(list_cache); } @@ -615,25 +612,21 @@ void client_manager_deinit(void) { client_close_all(); - free(clients); - client_max_connections = 0; } void client_manager_expire(void) { - unsigned int i; - - for (i = 0; i < client_max_connections; i++) { - if (clients[i].fd >= 0) { - if (clients[i].expired) { - DEBUG("client %i: expired\n", i); - client_close(&(clients[i])); - } else if (time(NULL) - clients[i].lastTime > - client_timeout) { - DEBUG("client %i: timeout\n", i); - client_close(&(clients[i])); - } + struct client *client, *n; + + list_for_each_entry_safe(client, n, &clients, siblings) { + if (client->expired) { + DEBUG("client %i: expired\n", client->num); + client_close(client); + } else if (time(NULL) - client->lastTime > + client_timeout) { + DEBUG("client %i: timeout\n", client->num); + client_close(client); } } } @@ -688,17 +681,11 @@ static void client_write_deferred(struct client *client) static struct client *client_by_fd(int fd) { - static unsigned int i; - - assert(fd >= 0); - - if (i < client_max_connections && clients[i].fd >= 0 && - clients[i].fd == fd) - return &clients[i]; + struct client *client; - for (i = 0; i < client_max_connections; i++) - if (clients[i].fd == fd) - return &clients[i]; + list_for_each_entry(client, &clients, siblings) + if (client->fd == fd) + return client; return NULL; } @@ -742,8 +729,7 @@ static void client_write_output(struct client *client) ssize_t ret; struct sllnode *buf; - if (client->fd < 0 || client->expired || - !client->send_buf_used) + if (client->expired || !client->send_buf_used) return; if ((buf = client->deferred_send)) { -- cgit v1.2.3 From 4184435c697ba953e0a224da5d8f7f4e880c1644 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:03:49 +0200 Subject: client: don't free client resources except in client_close() All of the client's resources are freed in client_close(). It is enough to set the "expired" flag, no need to duplicate lots of destruction code again and again. --- src/client.c | 15 --------------- 1 file changed, 15 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 2dbc9d0fd..3065a6940 100644 --- a/src/client.c +++ b/src/client.c @@ -667,14 +667,6 @@ static void client_write_deferred(struct client *client) /* cause client to close */ DEBUG("client %i: problems flushing buffer\n", client->num); - buf = client->deferred_send; - do { - struct sllnode *prev = buf; - buf = buf->next; - free(prev); - } while (buf); - client->deferred_send = NULL; - client->deferred_bytes = 0; client->expired = 1; } } @@ -744,13 +736,6 @@ static void client_write_output(struct client *client) (unsigned long)client_max_output_buffer_size); /* cause client to close */ client->expired = 1; - do { - struct sllnode *prev = buf; - buf = buf->next; - free(prev); - } while (buf); - client->deferred_send = NULL; - client->deferred_bytes = 0; } else { while (buf->next) buf = buf->next; -- cgit v1.2.3 From 7ff63fec2ca1c2c45d28a1907ab1e08e0356996e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:03:51 +0200 Subject: client: moved code to client_defer_output() Split the large function client_write_output() into two parts; this is the first code moving patch. --- src/client.c | 47 +++++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 20 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 3065a6940..586d4d212 100644 --- a/src/client.c +++ b/src/client.c @@ -716,33 +716,40 @@ int client_print(int fd, const char *buffer, size_t buflen) return 0; } +static void client_defer_output(struct client *client, + const void *data, size_t length) +{ + struct sllnode *buf = client->deferred_send; + + assert(client->deferred_send != NULL); + + client->deferred_bytes += sizeof(struct sllnode) + length; + if (client->deferred_bytes > client_max_output_buffer_size) { + ERROR("client %i: output buffer size (%lu) is " + "larger than the max (%lu)\n", + client->num, + (unsigned long)client->deferred_bytes, + (unsigned long)client_max_output_buffer_size); + /* cause client to close */ + client->expired = 1; + } else { + while (buf->next) + buf = buf->next; + buf->next = new_sllnode(data, length); + } +} + static void client_write_output(struct client *client) { ssize_t ret; - struct sllnode *buf; if (client->expired || !client->send_buf_used) return; - if ((buf = client->deferred_send)) { - client->deferred_bytes += sizeof(struct sllnode) - + client->send_buf_used; - if (client->deferred_bytes > - client_max_output_buffer_size) { - ERROR("client %i: output buffer size (%lu) is " - "larger than the max (%lu)\n", - client->num, - (unsigned long)client->deferred_bytes, - (unsigned long)client_max_output_buffer_size); - /* cause client to close */ - client->expired = 1; - } else { - while (buf->next) - buf = buf->next; - buf->next = new_sllnode(client->send_buf, - client->send_buf_used); - } - } else { + if (client->deferred_send != NULL) + client_defer_output(client, client->send_buf, + client->send_buf_used); + else { if ((ret = write(client->fd, client->send_buf, client->send_buf_used)) < 0) { if (errno == EAGAIN || errno == EINTR) { -- cgit v1.2.3 From 3682efe7a705043f9005c456cfc806aeefa788ff Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:03:54 +0200 Subject: client: return early on error in client_defer_output() Exit the function when an error occurs, and move the rest of the following code one indent level left. --- src/client.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 586d4d212..6457fb373 100644 --- a/src/client.c +++ b/src/client.c @@ -719,7 +719,7 @@ int client_print(int fd, const char *buffer, size_t buflen) static void client_defer_output(struct client *client, const void *data, size_t length) { - struct sllnode *buf = client->deferred_send; + struct sllnode *buf; assert(client->deferred_send != NULL); @@ -732,11 +732,13 @@ static void client_defer_output(struct client *client, (unsigned long)client_max_output_buffer_size); /* cause client to close */ client->expired = 1; - } else { - while (buf->next) - buf = buf->next; - buf->next = new_sllnode(data, length); + return; } + + buf = client->deferred_send; + while (buf->next) + buf = buf->next; + buf->next = new_sllnode(data, length); } static void client_write_output(struct client *client) -- cgit v1.2.3 From d6c92ea1348730de5ed8d31b40c92eb5b8e71944 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:03:56 +0200 Subject: client: client_defer_output() can create the first defer buffer client_defer_output() was designed to add new buffers to an existing deferred_send buffer. Tweak it and allow it to create a new buffer list. --- src/client.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 6457fb373..531177ab2 100644 --- a/src/client.c +++ b/src/client.c @@ -719,7 +719,7 @@ int client_print(int fd, const char *buffer, size_t buflen) static void client_defer_output(struct client *client, const void *data, size_t length) { - struct sllnode *buf; + struct sllnode **buf_r; assert(client->deferred_send != NULL); @@ -735,10 +735,10 @@ static void client_defer_output(struct client *client, return; } - buf = client->deferred_send; - while (buf->next) - buf = buf->next; - buf->next = new_sllnode(data, length); + buf_r = &client->deferred_send; + while (*buf_r != NULL) + buf_r = &(*buf_r)->next; + *buf_r = new_sllnode(data, length); } static void client_write_output(struct client *client) -- cgit v1.2.3 From e3293d40147f440cc520f77084a3216064a9b4dd Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:03:58 +0200 Subject: client: moved code to client_write() Move the second part of client_write_output() into a separate function. --- src/client.c | 54 ++++++++++++++++++++++++++++-------------------------- 1 file changed, 28 insertions(+), 26 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 531177ab2..d681f8b0b 100644 --- a/src/client.c +++ b/src/client.c @@ -741,41 +741,43 @@ static void client_defer_output(struct client *client, *buf_r = new_sllnode(data, length); } -static void client_write_output(struct client *client) +static void client_write(struct client *client, + const char *data, size_t length) { ssize_t ret; + assert(client->deferred_send == NULL); + + if ((ret = write(client->fd, data, length)) < 0) { + if (errno == EAGAIN || errno == EINTR) { + client->deferred_send = new_sllnode(data, length); + } else { + DEBUG("client %i: problems writing\n", client->num); + client->expired = 1; + return; + } + } else if ((size_t)ret < client->send_buf_used) { + client->deferred_send = new_sllnode(data + ret, length - ret); + } + + if (client->deferred_send) { + DEBUG("client %i: buffer created\n", client->num); + client->deferred_bytes = + client->deferred_send->size + + sizeof(struct sllnode); + } +} + +static void client_write_output(struct client *client) +{ if (client->expired || !client->send_buf_used) return; if (client->deferred_send != NULL) client_defer_output(client, client->send_buf, client->send_buf_used); - else { - if ((ret = write(client->fd, client->send_buf, - client->send_buf_used)) < 0) { - if (errno == EAGAIN || errno == EINTR) { - client->deferred_send = - new_sllnode(client->send_buf, - client->send_buf_used); - } else { - DEBUG("client %i: problems writing\n", - client->num); - client->expired = 1; - return; - } - } else if ((size_t)ret < client->send_buf_used) { - client->deferred_send = - new_sllnode(client->send_buf + ret, - client->send_buf_used - ret); - } - if (client->deferred_send) { - DEBUG("client %i: buffer created\n", client->num); - client->deferred_bytes = - client->deferred_send->size - + sizeof(struct sllnode); - } - } + else + client_write(client, client->send_buf, client->send_buf_used); client->send_buf_used = 0; } -- cgit v1.2.3 From 592d7484ce76fde36c78dc85a6099149c42b734c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:20:04 +0200 Subject: client: use client_defer_output() in client_write() Eliminate duplicated code, call client_defer_output() which we splitted from client_write_output() earlier. --- src/client.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index d681f8b0b..4bb11ba5a 100644 --- a/src/client.c +++ b/src/client.c @@ -750,22 +750,18 @@ static void client_write(struct client *client, if ((ret = write(client->fd, data, length)) < 0) { if (errno == EAGAIN || errno == EINTR) { - client->deferred_send = new_sllnode(data, length); + client_defer_output(client, data, length); } else { DEBUG("client %i: problems writing\n", client->num); client->expired = 1; return; } } else if ((size_t)ret < client->send_buf_used) { - client->deferred_send = new_sllnode(data + ret, length - ret); + client_defer_output(client, data + ret, length - ret); } - if (client->deferred_send) { + if (client->deferred_send) DEBUG("client %i: buffer created\n", client->num); - client->deferred_bytes = - client->deferred_send->size - + sizeof(struct sllnode); - } } static void client_write_output(struct client *client) -- cgit v1.2.3 From 1cb70dcaf7d733c4bbf55d6071d7eabd28651814 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:20:10 +0200 Subject: client: select() errors are fatal Previously, when select() failed, we assumed that there was an invalid file descriptor in one of the client structs. Thus we tried select() one by one. This is bogus, because we should never have invalid file descriptors. Remove it, and make select() errors fatal. --- src/client.c | 36 ++++++------------------------------ 1 file changed, 6 insertions(+), 30 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 4bb11ba5a..6ee9b988d 100644 --- a/src/client.c +++ b/src/client.c @@ -469,26 +469,6 @@ static void client_manager_register_write_fd(fd_set * fds, int *fdmax) } } -static void closeNextErroredInterface(void) -{ - struct client *client, *n; - fd_set fds; - struct timeval tv; - - tv.tv_sec = 0; - tv.tv_usec = 0; - - list_for_each_entry_safe(client, n, &clients, siblings) { - FD_ZERO(&fds); - FD_SET(client->fd, &fds); - if (select(client->fd + 1, - &fds, NULL, NULL, &tv) < 0) { - client_close(client); - return; - } - } -} - int client_manager_io(void) { fd_set rfds; @@ -508,19 +488,15 @@ int client_manager_io(void) registered_IO_add_fds(&fdmax, &rfds, &wfds, &efds); selret = select(fdmax + 1, &rfds, &wfds, &efds, NULL); - if (selret < 0 && errno == EINTR) - break; - - registered_IO_consume_fds(&selret, &rfds, &wfds, &efds); - - if (selret == 0) - break; - if (selret < 0) { - closeNextErroredInterface(); - continue; + if (errno == EINTR) + break; + + FATAL("select() failed: %s\n", strerror(errno)); } + registered_IO_consume_fds(&selret, &rfds, &wfds, &efds); + getConnections(&rfds); list_for_each_entry_safe(client, n, &clients, siblings) { -- cgit v1.2.3 From 827d346899cf0d0d95f8320660944c8f38488b57 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:20:10 +0200 Subject: client: no while loop in client_manager_io() The last patch removed the "continue" directive, and now the while loop is without function. Remove it. Also make client_manager_io() return 0. --- src/client.c | 54 ++++++++++++++++++++++++------------------------------ 1 file changed, 24 insertions(+), 30 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 6ee9b988d..f03dd407b 100644 --- a/src/client.c +++ b/src/client.c @@ -476,47 +476,41 @@ int client_manager_io(void) fd_set efds; struct client *client, *n; int selret; - int fdmax; + int fdmax = 0; - while (1) { - fdmax = 0; + FD_ZERO( &efds ); + client_manager_register_read_fd(&rfds, &fdmax); + client_manager_register_write_fd(&wfds, &fdmax); - FD_ZERO( &efds ); - client_manager_register_read_fd(&rfds, &fdmax); - client_manager_register_write_fd(&wfds, &fdmax); + registered_IO_add_fds(&fdmax, &rfds, &wfds, &efds); - registered_IO_add_fds(&fdmax, &rfds, &wfds, &efds); + selret = select(fdmax + 1, &rfds, &wfds, &efds, NULL); + if (selret < 0) { + if (errno == EINTR) + return 0; - selret = select(fdmax + 1, &rfds, &wfds, &efds, NULL); - if (selret < 0) { - if (errno == EINTR) - break; - - FATAL("select() failed: %s\n", strerror(errno)); - } + FATAL("select() failed: %s\n", strerror(errno)); + } - registered_IO_consume_fds(&selret, &rfds, &wfds, &efds); + registered_IO_consume_fds(&selret, &rfds, &wfds, &efds); - getConnections(&rfds); + getConnections(&rfds); - list_for_each_entry_safe(client, n, &clients, siblings) { - if (FD_ISSET(client->fd, &rfds)) { - if (COMMAND_RETURN_KILL == - client_read(client)) { - return COMMAND_RETURN_KILL; - } - client->lastTime = time(NULL); - } - if (FD_ISSET(client->fd, &wfds)) { - client_write_deferred(client); - client->lastTime = time(NULL); + list_for_each_entry_safe(client, n, &clients, siblings) { + if (FD_ISSET(client->fd, &rfds)) { + if (COMMAND_RETURN_KILL == + client_read(client)) { + return COMMAND_RETURN_KILL; } + client->lastTime = time(NULL); + } + if (FD_ISSET(client->fd, &wfds)) { + client_write_deferred(client); + client->lastTime = time(NULL); } - - break; } - return 1; + return 0; } void client_manager_init(void) -- cgit v1.2.3 From 9567f849e514940da6c2c6da52dde932b7b2fb48 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:20:10 +0200 Subject: client: moved "expired" accesses into inline function Hiding this flag allows us later to remove it easily. --- src/client.c | 42 +++++++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index f03dd407b..f1e3685c0 100644 --- a/src/client.c +++ b/src/client.c @@ -131,6 +131,16 @@ static void set_send_buf_size(struct client *client) } } +static inline int client_is_expired(const struct client *client) +{ + return client->expired; +} + +static inline void client_set_expired(struct client *client) +{ + client->expired = 1; +} + static void client_init(struct client *client, int fd) { static unsigned int next_client_num; @@ -307,19 +317,25 @@ static int client_process_line(struct client *client) if (client->cmd_list_OK >= 0) { if (strcmp(line, CLIENT_LIST_MODE_END) == 0) { + int expired; + DEBUG("client %i: process command " "list\n", client->num); ret = processListOfCommands(client->fd, &(client->permission), - &(client->expired), + &(expired), client->cmd_list_OK, client->cmd_list); DEBUG("client %i: process command " "list returned %i\n", client->num, ret); + + if (expired) + client_set_expired(client); + if (ret == 0) commandSuccess(client->fd); else if (ret == COMMAND_RETURN_CLOSE - || client->expired) + || client_is_expired(client)) client_close(client); client_write_output(client); @@ -361,7 +377,7 @@ static int client_process_line(struct client *client) if (ret == 0) commandSuccess(client->fd); else if (ret == COMMAND_RETURN_CLOSE - || client->expired) { + || client_is_expired(client)) { client_close(client); } client_write_output(client); @@ -387,7 +403,7 @@ static int client_input_received(struct client *client, int bytesRead) *(buf_tail - 1) = '\0'; } ret = client_process_line(client); - if (client->expired) + if (client_is_expired(client)) return ret; client->bufferPos = client->bufferLength; } @@ -445,7 +461,7 @@ static void client_manager_register_read_fd(fd_set * fds, int *fdmax) addListenSocketsToFdSet(fds, fdmax); list_for_each_entry(client, &clients, siblings) { - if (!client->expired && !client->deferred_send) { + if (!client_is_expired(client) && !client->deferred_send) { FD_SET(client->fd, fds); if (*fdmax < client->fd) *fdmax = client->fd; @@ -460,7 +476,7 @@ static void client_manager_register_write_fd(fd_set * fds, int *fdmax) FD_ZERO(fds); list_for_each_entry(client, &clients, siblings) { - if (client->fd >= 0 && !client->expired + if (client->fd >= 0 && !client_is_expired(client) && client->deferred_send) { FD_SET(client->fd, fds); if (*fdmax < client->fd) @@ -590,7 +606,7 @@ void client_manager_expire(void) struct client *client, *n; list_for_each_entry_safe(client, n, &clients, siblings) { - if (client->expired) { + if (client_is_expired(client)) { DEBUG("client %i: expired\n", client->num); client_close(client); } else if (time(NULL) - client->lastTime > @@ -637,7 +653,7 @@ static void client_write_deferred(struct client *client) /* cause client to close */ DEBUG("client %i: problems flushing buffer\n", client->num); - client->expired = 1; + client_set_expired(client); } } @@ -664,10 +680,10 @@ int client_print(int fd, const char *buffer, size_t buflen) return -1; /* if fd isn't found or client is going to be closed, do nothing */ - if (client->expired) + if (client_is_expired(client)) return 0; - while (buflen > 0 && !client->expired) { + while (buflen > 0 && !client_is_expired(client)) { size_t left; assert(client->send_buf_size >= client->send_buf_used); @@ -701,7 +717,7 @@ static void client_defer_output(struct client *client, (unsigned long)client->deferred_bytes, (unsigned long)client_max_output_buffer_size); /* cause client to close */ - client->expired = 1; + client_set_expired(client); return; } @@ -723,7 +739,7 @@ static void client_write(struct client *client, client_defer_output(client, data, length); } else { DEBUG("client %i: problems writing\n", client->num); - client->expired = 1; + client_set_expired(client); return; } } else if ((size_t)ret < client->send_buf_used) { @@ -736,7 +752,7 @@ static void client_write(struct client *client, static void client_write_output(struct client *client) { - if (client->expired || !client->send_buf_used) + if (client_is_expired(client) || !client->send_buf_used) return; if (client->deferred_send != NULL) -- cgit v1.2.3 From 9e35e50f856038e231e06a5f3d85ff1545053438 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:20:10 +0200 Subject: client: replace "expired" flag with fd==-1 Why waste 4 bytes for a flag which we can hide in another variable. --- src/client.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index f1e3685c0..ba15a90de 100644 --- a/src/client.c +++ b/src/client.c @@ -66,7 +66,7 @@ struct client { char buffer[CLIENT_MAX_BUFFER_LENGTH]; size_t bufferLength; size_t bufferPos; - int fd; /* file descriptor */ + int fd; /* file descriptor; -1 if expired */ int permission; time_t lastTime; struct strnode *cmd_list; /* for when in list mode */ @@ -76,8 +76,6 @@ struct client { int cmd_list_dup; /* has the cmd_list been copied to private space? */ struct sllnode *deferred_send; /* for output if client is slow */ size_t deferred_bytes; /* mem deferred_send consumes */ - int expired; /* set whether this client should be closed on next - check of old clients */ unsigned int num; /* client number */ char *send_buf; @@ -133,12 +131,15 @@ static void set_send_buf_size(struct client *client) static inline int client_is_expired(const struct client *client) { - return client->expired; + return client->fd < 0; } static inline void client_set_expired(struct client *client) { - client->expired = 1; + if (client->fd >= 0) { + xclose(client->fd); + client->fd = -1; + } } static void client_init(struct client *client, int fd) @@ -156,7 +157,6 @@ static void client_init(struct client *client, int fd) client->cmd_list = NULL; client->cmd_list_tail = NULL; client->deferred_send = NULL; - client->expired = 0; client->deferred_bytes = 0; client->num = next_client_num++; client->send_buf_used = 0; -- cgit v1.2.3 From 692cfc5c36ddff07888d1c2e7ecefda8be8ead47 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:20:10 +0200 Subject: client: moved code to sockaddr_to_tmp_string() Unclutter the client_new() constructor by moving unrelated complex code into a separate function. --- src/client.c | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index ba15a90de..f34d64b88 100644 --- a/src/client.c +++ b/src/client.c @@ -258,16 +258,10 @@ static void client_close(struct client *client) free(client); } -void client_new(int fd, const struct sockaddr *addr) +static const char * +sockaddr_to_tmp_string(const struct sockaddr *addr) { const char *hostname; - struct client *client; - - if (num_clients >= client_max_connections) { - ERROR("Max Connections Reached!\n"); - xclose(fd); - return; - } switch (addr->sa_family) { #ifdef HAVE_TCP @@ -303,11 +297,25 @@ void client_new(int fd, const struct sockaddr *addr) hostname = "unknown"; } + return hostname; +} + +void client_new(int fd, const struct sockaddr *addr) +{ + struct client *client; + + if (num_clients >= client_max_connections) { + ERROR("Max Connections Reached!\n"); + xclose(fd); + return; + } + client = xcalloc(1, sizeof(*client)); list_add(&client->siblings, &clients); ++num_clients; client_init(client, fd); - SECURE("client %i: opened from %s\n", client->num, hostname); + SECURE("client %i: opened from %s\n", client->num, + sockaddr_to_tmp_string(addr)); } static int client_process_line(struct client *client) -- cgit v1.2.3 From 557679eba7e6daae37e90c645b1e69174fb1456c Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 28 Aug 2008 20:23:22 +0200 Subject: client: more assertions --- src/client.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'src') diff --git a/src/client.c b/src/client.c index f34d64b88..51f3e8e3b 100644 --- a/src/client.c +++ b/src/client.c @@ -146,6 +146,8 @@ static void client_init(struct client *client, int fd) { static unsigned int next_client_num; + assert(fd >= 0); + client->cmd_list_size = 0; client->cmd_list_dup = 0; client->cmd_list_OK = -1; @@ -632,6 +634,8 @@ static void client_write_deferred(struct client *client) buf = client->deferred_send; while (buf) { + assert(buf->size > 0); + ret = write(client->fd, buf->data, buf->size); if (ret < 0) break; @@ -715,6 +719,7 @@ static void client_defer_output(struct client *client, { struct sllnode **buf_r; + assert(length > 0); assert(client->deferred_send != NULL); client->deferred_bytes += sizeof(struct sllnode) + length; @@ -740,6 +745,7 @@ static void client_write(struct client *client, { ssize_t ret; + assert(length > 0); assert(client->deferred_send == NULL); if ((ret = write(client->fd, data, length)) < 0) { -- cgit v1.2.3 From 421fced5f9484a288fdc4f3ab33dfd4bfbfb7e84 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 06:17:54 +0200 Subject: client: removed superfluous assertion client_defer_output() was modified so that it can create the deferred_send list. With this patch, the assertion on "deferred_send!=NULL" has become invalid. Remove it. --- src/client.c | 1 - 1 file changed, 1 deletion(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 51f3e8e3b..3a96e4caf 100644 --- a/src/client.c +++ b/src/client.c @@ -720,7 +720,6 @@ static void client_defer_output(struct client *client, struct sllnode **buf_r; assert(length > 0); - assert(client->deferred_send != NULL); client->deferred_bytes += sizeof(struct sllnode) + length; if (client->deferred_bytes > client_max_output_buffer_size) { -- cgit v1.2.3 From 4ad71d6f79dfd8700441a5b3334e10ae5ab9f157 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:36:38 +0200 Subject: client: added global "expired" flag Patch bdeb8e14 ("client: moved "expired" accesses into inline function") was created under the wrong assumption that processListOfCommands() could modify the expired flag, which is not the case. Although "expired" is a non-const pointer, processListOfCommands() just reads it, using it as the break condition in a "while" loop. I will address this issue with a better overall solution, but for now provide a pointer to a global "expired" flag. --- src/client.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 3a96e4caf..bce0acd5e 100644 --- a/src/client.c +++ b/src/client.c @@ -134,12 +134,16 @@ static inline int client_is_expired(const struct client *client) return client->fd < 0; } +static int global_expired; + static inline void client_set_expired(struct client *client) { if (client->fd >= 0) { xclose(client->fd); client->fd = -1; } + + global_expired = 1; } static void client_init(struct client *client, int fd) @@ -327,21 +331,18 @@ static int client_process_line(struct client *client) if (client->cmd_list_OK >= 0) { if (strcmp(line, CLIENT_LIST_MODE_END) == 0) { - int expired; - DEBUG("client %i: process command " "list\n", client->num); + + global_expired = 0; ret = processListOfCommands(client->fd, &(client->permission), - &(expired), + &global_expired, client->cmd_list_OK, client->cmd_list); DEBUG("client %i: process command " "list returned %i\n", client->num, ret); - if (expired) - client_set_expired(client); - if (ret == 0) commandSuccess(client->fd); else if (ret == COMMAND_RETURN_CLOSE -- cgit v1.2.3 From 69af10a006bf9e67f4689642e06bae24f67e873d Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:36:40 +0200 Subject: client: check "expired" after command execution The old code tried to write a response to the client, without even checking if it was already closed. Now that we have added more assertions, these may fail... perform the "expired" check earlier. --- src/client.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index bce0acd5e..6a43b9586 100644 --- a/src/client.c +++ b/src/client.c @@ -343,11 +343,14 @@ static int client_process_line(struct client *client) DEBUG("client %i: process command " "list returned %i\n", client->num, ret); + if (ret == COMMAND_RETURN_CLOSE || + client_is_expired(client)) { + client_close(client); + return COMMAND_RETURN_CLOSE; + } + if (ret == 0) commandSuccess(client->fd); - else if (ret == COMMAND_RETURN_CLOSE - || client_is_expired(client)) - client_close(client); client_write_output(client); free_cmd_list(client->cmd_list); @@ -385,12 +388,16 @@ static int client_process_line(struct client *client) &(client->permission), line); DEBUG("client %i: command returned %i\n", client->num, ret); - if (ret == 0) - commandSuccess(client->fd); - else if (ret == COMMAND_RETURN_CLOSE - || client_is_expired(client)) { + + if (ret == COMMAND_RETURN_CLOSE || + client_is_expired(client)) { client_close(client); + return COMMAND_RETURN_CLOSE; } + + if (ret == 0) + commandSuccess(client->fd); + client_write_output(client); } } -- cgit v1.2.3 From 0e645468c6249800f573efaa4bca94753245d7e0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Fri, 29 Aug 2008 09:36:40 +0200 Subject: client: reorder function declarations Change the order of function declarations in client.h, to make it well arranged and readable. --- src/client.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/client.h b/src/client.h index 64639b2af..a4cb4a902 100644 --- a/src/client.h +++ b/src/client.h @@ -22,11 +22,12 @@ #include "os_compat.h" void client_manager_init(void); -void client_new(int fd, const struct sockaddr *addr); void client_manager_deinit(void); +int client_manager_io(void); void client_manager_expire(void); -int client_print(int fd, const char *buffer, size_t len); -int client_manager_io(void); +void client_new(int fd, const struct sockaddr *addr); + +int client_print(int fd, const char *buffer, size_t len); #endif -- cgit v1.2.3 From 57b17b7e6585fc9dfdaee10e31441e39354a4422 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 6 Sep 2008 15:28:31 +0200 Subject: dbUtils, playlist, directory: pass constant pointers The usual bunch of const pointer conversions. --- src/dbUtils.c | 19 ++++++++++--------- src/dbUtils.h | 18 +++++++++--------- src/directory.c | 4 ++-- src/directory.h | 4 ++-- src/playlist.c | 14 +++++++------- src/playlist.h | 16 ++++++++-------- 6 files changed, 38 insertions(+), 37 deletions(-) (limited to 'src') diff --git a/src/dbUtils.c b/src/dbUtils.c index d39c9908c..8c499ec7e 100644 --- a/src/dbUtils.c +++ b/src/dbUtils.c @@ -80,7 +80,8 @@ static int searchInDirectory(int fd, Song * song, void *data) return 0; } -int searchForSongsIn(int fd, char *name, int numItems, LocateTagItem * items) +int searchForSongsIn(int fd, const char *name, int numItems, + LocateTagItem * items) { int ret; int i; @@ -118,7 +119,7 @@ static int findInDirectory(int fd, Song * song, void *data) return 0; } -int findSongsIn(int fd, char *name, int numItems, LocateTagItem * items) +int findSongsIn(int fd, const char *name, int numItems, LocateTagItem * items) { LocateTagItemArray array; @@ -148,7 +149,7 @@ static int searchStatsInDirectory(mpd_unused int fd, Song * song, void *data) return 0; } -int searchStatsForSongsIn(int fd, char *name, int numItems, +int searchStatsForSongsIn(int fd, const char *name, int numItems, LocateTagItem * items) { SearchStats stats; @@ -166,7 +167,7 @@ int searchStatsForSongsIn(int fd, char *name, int numItems, return ret; } -int printAllIn(int fd, char *name) +int printAllIn(int fd, const char *name) { return traverseAllIn(fd, name, printSongInDirectory, printDirectoryInDirectory, NULL); @@ -185,12 +186,12 @@ static int directoryAddSongToStoredPlaylist(int fd, Song *song, void *data) return 0; } -int addAllIn(int fd, char *name) +int addAllIn(int fd, const char *name) { return traverseAllIn(fd, name, directoryAddSongToPlaylist, NULL, NULL); } -int addAllInToStoredPlaylist(int fd, char *name, char *utf8file) +int addAllInToStoredPlaylist(int fd, const char *name, const char *utf8file) { return traverseAllIn(fd, name, directoryAddSongToStoredPlaylist, NULL, (void *)utf8file); @@ -211,13 +212,13 @@ static int sumSongTime(mpd_unused int fd, Song * song, void *data) return 0; } -int printInfoForAllIn(int fd, char *name) +int printInfoForAllIn(int fd, const char *name) { return traverseAllIn(fd, name, directoryPrintSongInfo, printDirectoryInDirectory, NULL); } -int countSongsIn(int fd, char *name) +int countSongsIn(int fd, const char *name) { int count = 0; void *ptr = (void *)&count; @@ -227,7 +228,7 @@ int countSongsIn(int fd, char *name) return count; } -unsigned long sumSongTimesIn(int fd, char *name) +unsigned long sumSongTimesIn(int fd, const char *name) { unsigned long dbPlayTime = 0; void *ptr = (void *)&dbPlayTime; diff --git a/src/dbUtils.h b/src/dbUtils.h index f2237eab7..89b69bfc3 100644 --- a/src/dbUtils.h +++ b/src/dbUtils.h @@ -21,25 +21,25 @@ #include "locate.h" -int printAllIn(int fd, char *name); +int printAllIn(int fd, const char *name); -int addAllIn(int fd, char *name); +int addAllIn(int fd, const char *name); -int addAllInToStoredPlaylist(int fd, char *name, char *utf8file); +int addAllInToStoredPlaylist(int fd, const char *name, const char *utf8file); -int printInfoForAllIn(int fd, char *name); +int printInfoForAllIn(int fd, const char *name); -int searchForSongsIn(int fd, char *name, int numItems, +int searchForSongsIn(int fd, const char *name, int numItems, LocateTagItem * items); -int findSongsIn(int fd, char *name, int numItems, LocateTagItem * items); +int findSongsIn(int fd, const char *name, int numItems, LocateTagItem * items); -int searchStatsForSongsIn(int fd, char *name, int numItems, +int searchStatsForSongsIn(int fd, const char *name, int numItems, LocateTagItem * items); -int countSongsIn(int fd, char *name); +int countSongsIn(int fd, const char *name); -unsigned long sumSongTimesIn(int fd, char *name); +unsigned long sumSongTimesIn(int fd, const char *name); int listAllUniqueTags(int fd, int type, int numConditiionals, LocateTagItem * conditionals); diff --git a/src/directory.c b/src/directory.c index 33a862ddd..32db02a9b 100644 --- a/src/directory.c +++ b/src/directory.c @@ -1219,7 +1219,7 @@ static int traverseAllInSubDirectory(int fd, Directory * directory, return errFlag; } -int traverseAllIn(int fd, char *name, +int traverseAllIn(int fd, const char *name, int (*forEachSong) (int, Song *, void *), int (*forEachDir) (int, Directory *, void *), void *data) { @@ -1303,7 +1303,7 @@ static Song *getSongDetails(const char *file, const char **shortnameRet, return (Song *) song; } -Song *getSongFromDB(char *file) +Song *getSongFromDB(const char *file) { return getSongDetails(file, NULL, NULL); } diff --git a/src/directory.h b/src/directory.h index acb173fc0..00b902510 100644 --- a/src/directory.h +++ b/src/directory.h @@ -60,11 +60,11 @@ int readDirectoryDB(void); void updateMp3Directory(void); -Song *getSongFromDB(char *file); +Song *getSongFromDB(const char *file); time_t getDbModTime(void); -int traverseAllIn(int fd, char *name, +int traverseAllIn(int fd, const char *name, int (*forEachSong) (int, Song *, void *), int (*forEachDir) (int, Directory *, void *), void *data); diff --git a/src/playlist.c b/src/playlist.c index 89b4fdd99..629742aa3 100644 --- a/src/playlist.c +++ b/src/playlist.c @@ -238,7 +238,7 @@ void clearPlaylist(void) incrPlaylistVersion(); } -int clearStoredPlaylist(int fd, char *utf8file) +int clearStoredPlaylist(int fd, const char *utf8file) { return removeAllFromStoredPlaylistByPath(fd, utf8file); } @@ -597,7 +597,7 @@ static int clear_queue(void) return playlist.queued; } -int addToPlaylist(int fd, char *url, int *added_id) +int addToPlaylist(int fd, const char *url, int *added_id) { Song *song; @@ -615,7 +615,7 @@ int addToPlaylist(int fd, char *url, int *added_id) return addSongToPlaylist(fd, song, added_id); } -int addToStoredPlaylist(int fd, char *url, char *utf8file) +int addToStoredPlaylist(int fd, const char *url, const char *utf8file) { Song *song; @@ -1311,7 +1311,7 @@ int shufflePlaylist(mpd_unused int fd) return 0; } -int deletePlaylist(int fd, char *utf8file) +int deletePlaylist(int fd, const char *utf8file) { char path_max_tmp[MPD_PATH_MAX]; @@ -1332,7 +1332,7 @@ int deletePlaylist(int fd, char *utf8file) return 0; } -int savePlaylist(int fd, char *utf8file) +int savePlaylist(int fd, const char *utf8file) { FILE *fp; int i; @@ -1445,7 +1445,7 @@ int getPlaylistSongId(int song) return playlist.positionToId[song]; } -int PlaylistInfo(int fd, char *utf8file, int detail) +int PlaylistInfo(int fd, const char *utf8file, int detail) { ListNode *node; List *list; @@ -1477,7 +1477,7 @@ int PlaylistInfo(int fd, char *utf8file, int detail) return 0; } -int loadPlaylist(int fd, char *utf8file) +int loadPlaylist(int fd, const char *utf8file) { ListNode *node; List *list; diff --git a/src/playlist.h b/src/playlist.h index 629bfa057..c144d4ab4 100644 --- a/src/playlist.h +++ b/src/playlist.h @@ -38,11 +38,11 @@ void savePlaylistState(FILE *); void clearPlaylist(void); -int clearStoredPlaylist(int fd, char *utf8file); +int clearStoredPlaylist(int fd, const char *utf8file); -int addToPlaylist(int fd, char *file, int *added_id); +int addToPlaylist(int fd, const char *file, int *added_id); -int addToStoredPlaylist(int fd, char *file, char *utf8file); +int addToStoredPlaylist(int fd, const char *file, const char *utf8file); int addSongToPlaylist(int fd, Song * song, int *added_id); @@ -76,11 +76,11 @@ void previousSongInPlaylist(void); int shufflePlaylist(int fd); -int savePlaylist(int fd, char *utf8file); +int savePlaylist(int fd, const char *utf8file); -int deletePlaylist(int fd, char *utf8file); +int deletePlaylist(int fd, const char *utf8file); -int deletePlaylistById(int fd, char *utf8file); +int deletePlaylistById(int fd, const char *utf8file); void deleteASongFromPlaylist(Song * song); @@ -92,7 +92,7 @@ int swapSongsInPlaylist(int fd, int song1, int song2); int swapSongsInPlaylistById(int fd, int id1, int id2); -int loadPlaylist(int fd, char *utf8file); +int loadPlaylist(int fd, const char *utf8file); int getPlaylistRepeatStatus(void); @@ -120,7 +120,7 @@ int playlistChanges(int fd, mpd_uint32 version); int playlistChangesPosId(int fd, mpd_uint32 version); -int PlaylistInfo(int fd, char *utf8file, int detail); +int PlaylistInfo(int fd, const char *utf8file, int detail); void searchForSongsInPlaylist(int fd, int numItems, LocateTagItem * items); -- cgit v1.2.3 From b3a40661ffb4a0da1aefc7e7151bbf9e82f23cef Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:37:04 +0200 Subject: playlist: added is_valid_playlist_name() The function valid_playlist_name() checks the name, but it insists on reporting an eventual error to the client. The new function is_valid_playlist_name() is more generic: it just returns a boolean, and does not care what the caller will use it for. The old function valid_playlist_name() will be removed later. --- src/playlist.c | 11 ++++++++--- src/playlist.h | 2 ++ 2 files changed, 10 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/playlist.c b/src/playlist.c index 629742aa3..1a286f67a 100644 --- a/src/playlist.c +++ b/src/playlist.c @@ -1553,11 +1553,16 @@ void findSongsInPlaylist(int fd, int numItems, LocateTagItem * items) * protocol (and compatibility with all clients) to support idiots who * put '\r' and '\n' in filenames isn't going to happen, either. */ +int is_valid_playlist_name(const char *utf8path) +{ + return strchr(utf8path, '/') == NULL && + strchr(utf8path, '\n') == NULL && + strchr(utf8path, '\r') == NULL; +} + int valid_playlist_name(int err_fd, const char *utf8path) { - if (strchr(utf8path, '/') || - strchr(utf8path, '\n') || - strchr(utf8path, '\r')) { + if (!is_valid_playlist_name(utf8path)) { commandError(err_fd, ACK_ERROR_ARG, "playlist name \"%s\" is " "invalid: playlist names may not contain slashes," " newlines or carriage returns", diff --git a/src/playlist.h b/src/playlist.h index c144d4ab4..83b02fd08 100644 --- a/src/playlist.h +++ b/src/playlist.h @@ -126,6 +126,8 @@ void searchForSongsInPlaylist(int fd, int numItems, LocateTagItem * items); void findSongsInPlaylist(int fd, int numItems, LocateTagItem * items); +int is_valid_playlist_name(const char *utf8path); + int valid_playlist_name(int err_fd, const char *utf8path); struct mpd_tag *playlist_current_tag(void); -- cgit v1.2.3 From 99a7a9306b0fd9cf3587b8c0bc36258cac0d4ef6 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:37:20 +0200 Subject: playlist: replaced run-time check with assertion The "fspath" argument of writeStoredPlaylistToPath() must never be NULL. There should be an assertion on that, instead of a run-time check. [ew: fspath => utf8path] --- src/storedPlaylist.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/storedPlaylist.c b/src/storedPlaylist.c index 332f99456..b62ca42e4 100644 --- a/src/storedPlaylist.c +++ b/src/storedPlaylist.c @@ -67,7 +67,9 @@ static int writeStoredPlaylistToPath(int fd, List *list, const char *utf8path) char *s; char path_max_tmp[MPD_PATH_MAX]; - if (!utf8path || !valid_playlist_name(fd, utf8path)) + assert(utf8path); + + if (!valid_playlist_name(fd, utf8path)) return -1; utf8_to_fs_playlist_path(path_max_tmp, utf8path); -- cgit v1.2.3 From ff06a92213c0d430f93552188cc569a5fa506e18 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:37:33 +0200 Subject: playlist: fix FILE* leak in appendSongToStoredPlaylistByPath() When an error occurs after the file has been opened, the function will never close the FILE object. --- src/storedPlaylist.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src') diff --git a/src/storedPlaylist.c b/src/storedPlaylist.c index b62ca42e4..ba0451f38 100644 --- a/src/storedPlaylist.c +++ b/src/storedPlaylist.c @@ -305,9 +305,11 @@ int appendSongToStoredPlaylistByPath(int fd, const char *utf8path, Song *song) if (fstat(fileno(file), &st) < 0) { commandError(fd, ACK_ERROR_NO_EXIST, "could not stat file " "\"%s\": %s", path_max_tmp, strerror(errno)); + while (fclose(file) != 0 && errno == EINTR); return -1; } if (st.st_size >= ((MPD_PATH_MAX+1) * playlist_max_length)) { + while (fclose(file) != 0 && errno == EINTR); commandError(fd, ACK_ERROR_PLAYLIST_MAX, "playlist is at the max size"); return -1; -- cgit v1.2.3 From 9e53c28046bae4a02387aba5ddaddc74f7bcf246 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:38:59 +0200 Subject: playlist: moved "repeat" and "random" value checks to command.c Client's input values should be validated by the command implementation, and the core libraries shouldn't talk to the client directly if possible. Thus, setPlaylistRepeatStatus() and setPlaylistRandomStatus() don't get the file descriptor, and cannot fail (return void). --- src/command.c | 20 ++++++++++++++++++-- src/playlist.c | 26 ++++++-------------------- src/playlist.h | 4 ++-- 3 files changed, 26 insertions(+), 24 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index bc646804e..77548664f 100644 --- a/src/command.c +++ b/src/command.c @@ -771,7 +771,15 @@ static int handleRepeat(int fd, mpd_unused int *permission, if (check_int(fd, &status, argv[1], need_integer) < 0) return -1; - return setPlaylistRepeatStatus(fd, status); + + if (status != 0 && status != 1) { + commandError(fd, ACK_ERROR_ARG, + "\"%i\" is not 0 or 1", status); + return -1; + } + + setPlaylistRepeatStatus(status); + return 0; } static int handleRandom(int fd, mpd_unused int *permission, @@ -781,7 +789,15 @@ static int handleRandom(int fd, mpd_unused int *permission, if (check_int(fd, &status, argv[1], need_integer) < 0) return -1; - return setPlaylistRandomStatus(fd, status); + + if (status != 0 && status != 1) { + commandError(fd, ACK_ERROR_ARG, + "\"%i\" is not 0 or 1", status); + return -1; + } + + setPlaylistRandomStatus(status); + return 0; } static int handleStats(int fd, mpd_unused int *permission, diff --git a/src/playlist.c b/src/playlist.c index 1a286f67a..6081f1fde 100644 --- a/src/playlist.c +++ b/src/playlist.c @@ -349,9 +349,9 @@ void readPlaylistState(FILE *fp) if (strcmp (&(buffer[strlen(PLAYLIST_STATE_FILE_REPEAT)]), "1") == 0) { - setPlaylistRepeatStatus(STDERR_FILENO, 1); + setPlaylistRepeatStatus(1); } else - setPlaylistRepeatStatus(STDERR_FILENO, 0); + setPlaylistRepeatStatus(0); } else if (strncmp (buffer, PLAYLIST_STATE_FILE_CROSSFADE, @@ -368,9 +368,9 @@ void readPlaylistState(FILE *fp) (buffer [strlen(PLAYLIST_STATE_FILE_RANDOM)]), "1") == 0) { - setPlaylistRandomStatus(STDERR_FILENO, 1); + setPlaylistRandomStatus(1); } else - setPlaylistRandomStatus(STDERR_FILENO, 0); + setPlaylistRandomStatus(0); } else if (strncmp(buffer, PLAYLIST_STATE_FILE_CURRENT, strlen(PLAYLIST_STATE_FILE_CURRENT)) == 0) { @@ -1043,21 +1043,14 @@ int getPlaylistRandomStatus(void) return playlist.random; } -int setPlaylistRepeatStatus(int fd, int status) +void setPlaylistRepeatStatus(int status) { - if (status != 0 && status != 1) { - commandError(fd, ACK_ERROR_ARG, "\"%i\" is not 0 or 1", status); - return -1; - } - if (playlist_state == PLAYLIST_STATE_PLAY) { if (playlist.repeat && !status && playlist.queued == 0) clear_queue(); } playlist.repeat = status; - - return 0; } int moveSongInPlaylist(int fd, int from, int to) @@ -1217,15 +1210,10 @@ static void randomizeOrder(int start, int end) DEBUG("%s:%d current: %d\n", __func__, __LINE__, playlist.current); } -int setPlaylistRandomStatus(int fd, int status) +void setPlaylistRandomStatus(int status) { int statusWas = playlist.random; - if (status != 0 && status != 1) { - commandError(fd, ACK_ERROR_ARG, "\"%i\" is not 0 or 1", status); - return -1; - } - playlist.random = status; if (status != statusWas) { @@ -1239,8 +1227,6 @@ int setPlaylistRandomStatus(int fd, int status) __func__,__LINE__,playlist.queued); } } - - return 0; } void previousSongInPlaylist(void) diff --git a/src/playlist.h b/src/playlist.h index 83b02fd08..cae38becf 100644 --- a/src/playlist.h +++ b/src/playlist.h @@ -96,11 +96,11 @@ int loadPlaylist(int fd, const char *utf8file); int getPlaylistRepeatStatus(void); -int setPlaylistRepeatStatus(int fd, int status); +void setPlaylistRepeatStatus(int status); int getPlaylistRandomStatus(void); -int setPlaylistRandomStatus(int fd, int status); +void setPlaylistRandomStatus(int status); int getPlaylistCurrentSong(void); -- cgit v1.2.3 From e29a971c9619a6e988dceb5335645f7ae10a527e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:39:19 +0200 Subject: playlist: showPlaylist() and shufflePlaylist() cannot fail Make them both return void. --- src/command.c | 6 ++++-- src/playlist.c | 8 ++------ src/playlist.h | 4 ++-- 3 files changed, 8 insertions(+), 10 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index 77548664f..14932b7d2 100644 --- a/src/command.c +++ b/src/command.c @@ -435,13 +435,15 @@ static int handleDeleteId(int fd, mpd_unused int *permission, static int handlePlaylist(int fd, mpd_unused int *permission, mpd_unused int argc, mpd_unused char *argv[]) { - return showPlaylist(fd); + showPlaylist(fd); + return 0; } static int handleShuffle(int fd, mpd_unused int *permission, mpd_unused int argc, mpd_unused char *argv[]) { - return shufflePlaylist(fd); + shufflePlaylist(fd); + return 0; } static int handleClear(mpd_unused int fd, mpd_unused int *permission, diff --git a/src/playlist.c b/src/playlist.c index 6081f1fde..7d782c1a1 100644 --- a/src/playlist.c +++ b/src/playlist.c @@ -243,7 +243,7 @@ int clearStoredPlaylist(int fd, const char *utf8file) return removeAllFromStoredPlaylistByPath(fd, utf8file); } -int showPlaylist(int fd) +void showPlaylist(int fd) { int i; char path_max_tmp[MPD_PATH_MAX]; @@ -252,8 +252,6 @@ int showPlaylist(int fd) fdprintf(fd, "%i:%s\n", i, get_song_url(path_max_tmp, playlist.songs[i])); } - - return 0; } void savePlaylistState(FILE *fp) @@ -1256,7 +1254,7 @@ void previousSongInPlaylist(void) play_order_num(prev_order_num, 0); } -int shufflePlaylist(mpd_unused int fd) +void shufflePlaylist(mpd_unused int fd) { int i; int ri; @@ -1293,8 +1291,6 @@ int shufflePlaylist(mpd_unused int fd) if (playlist_state == PLAYLIST_STATE_PLAY) queueNextSongInPlaylist(); } - - return 0; } int deletePlaylist(int fd, const char *utf8file) diff --git a/src/playlist.h b/src/playlist.h index cae38becf..3afe33da0 100644 --- a/src/playlist.h +++ b/src/playlist.h @@ -46,7 +46,7 @@ int addToStoredPlaylist(int fd, const char *file, const char *utf8file); int addSongToPlaylist(int fd, Song * song, int *added_id); -int showPlaylist(int fd); +void showPlaylist(int fd); int deleteFromPlaylist(int fd, int song); @@ -74,7 +74,7 @@ void syncPlayerAndPlaylist(void); void previousSongInPlaylist(void); -int shufflePlaylist(int fd); +void shufflePlaylist(int fd); int savePlaylist(int fd, const char *utf8file); -- cgit v1.2.3 From 6dcd7fea0e3cfc8f51097f11c0c84793584e8f0b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:39:31 +0200 Subject: playlist: don't pass "fd" to playlist.c functions The playlist library shouldn't talk to the client if possible. Introduce the "enum playlist_result" type which the caller (i.e. command.c) may use to generate an error message. --- src/command.c | 162 +++++++++++++++++++++++++++++++++---------- src/dbUtils.c | 4 +- src/playlist.c | 216 +++++++++++++++++++++++---------------------------------- src/playlist.h | 48 ++++++++----- 4 files changed, 245 insertions(+), 185 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index 14932b7d2..a4d785126 100644 --- a/src/command.c +++ b/src/command.c @@ -218,6 +218,53 @@ static int mpd_fprintf__ check_int(int fd, int *dst, return 0; } +static int print_playlist_result(int fd, enum playlist_result result) +{ + switch (result) { + case PLAYLIST_RESULT_SUCCESS: + return 0; + + case PLAYLIST_RESULT_ERRNO: + commandError(fd, ACK_ERROR_SYSTEM, strerror(errno)); + return -1; + + case PLAYLIST_RESULT_NO_SUCH_SONG: + commandError(fd, ACK_ERROR_NO_EXIST, "No such song"); + return -1; + + case PLAYLIST_RESULT_NO_SUCH_LIST: + commandError(fd, ACK_ERROR_NO_EXIST, "No such playlist"); + return -1; + + case PLAYLIST_RESULT_LIST_EXISTS: + commandError(fd, ACK_ERROR_NO_EXIST, + "Playlist already exists"); + return -1; + + case PLAYLIST_RESULT_BAD_NAME: + commandError(fd, ACK_ERROR_ARG, + "playlist name is invalid: " + "playlist names may not contain slashes," + " newlines or carriage returns"); + return -1; + + case PLAYLIST_RESULT_BAD_RANGE: + commandError(fd, ACK_ERROR_ARG, "Bad song index"); + return -1; + + case PLAYLIST_RESULT_NOT_PLAYING: + commandError(fd, ACK_ERROR_PLAYER_SYNC, "Not playing"); + return -1; + + case PLAYLIST_RESULT_TOO_LARGE: + commandError(fd, ACK_ERROR_PLAYLIST_MAX, + "playlist is at the max size"); + return -1; + } + + assert(0); +} + static void addCommand(const char *name, int reqPermission, int minargs, @@ -253,21 +300,25 @@ static int handlePlay(int fd, mpd_unused int *permission, int argc, char *argv[]) { int song = -1; + enum playlist_result result; if (argc == 2 && check_int(fd, &song, argv[1], need_positive) < 0) return -1; - return playPlaylist(fd, song, 0); + result = playPlaylist(song, 0); + return print_playlist_result(fd, result); } static int handlePlayId(int fd, mpd_unused int *permission, int argc, char *argv[]) { int id = -1; + enum playlist_result result; if (argc == 2 && check_int(fd, &id, argv[1], need_positive) < 0) return -1; - return playPlaylistById(fd, id, 0); + result = playPlaylistById(id, 0); + return print_playlist_result(fd, result); } static int handleStop(mpd_unused int fd, mpd_unused int *permission, @@ -281,11 +332,13 @@ static int handleCurrentSong(int fd, mpd_unused int *permission, mpd_unused int argc, mpd_unused char *argv[]) { int song = getPlaylistCurrentSong(); + enum playlist_result result; - if (song >= 0) { - return playlistInfo(fd, song); - } else + if (song < 0) return 0; + + result = playlistInfo(fd, song); + return print_playlist_result(fd, result); } static int handlePause(int fd, mpd_unused int *permission, @@ -382,54 +435,65 @@ static int handleAdd(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { char *path = argv[1]; + enum playlist_result result; if (isRemoteUrl(path)) - return addToPlaylist(fd, path, NULL); + return addToPlaylist(path, NULL); - return addAllIn(fd, path); + result = addAllIn(fd, path); + return print_playlist_result(fd, result); } static int handleAddId(int fd, mpd_unused int *permission, int argc, char *argv[]) { int added_id; - int ret = addToPlaylist(fd, argv[1], &added_id); - - if (!ret) { - if (argc == 3) { - int to; - if (check_int(fd, &to, argv[2], - check_integer, argv[2]) < 0) - return -1; - ret = moveSongInPlaylistById(fd, added_id, to); - if (ret) { /* move failed */ - deleteFromPlaylistById(fd, added_id); - return ret; - } + enum playlist_result result = addToPlaylist(argv[1], &added_id); + + if (result == PLAYLIST_RESULT_SUCCESS) + return result; + + if (argc == 3) { + int to; + if (check_int(fd, &to, argv[2], + check_integer, argv[2]) < 0) + return -1; + result = moveSongInPlaylistById(added_id, to); + if (result != PLAYLIST_RESULT_SUCCESS) { + int ret = print_playlist_result(fd, result); + deleteFromPlaylistById(added_id); + return ret; } - fdprintf(fd, "Id: %d\n", added_id); } - return ret; + + fdprintf(fd, "Id: %d\n", added_id); + return result; } static int handleDelete(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { int song; + enum playlist_result result; if (check_int(fd, &song, argv[1], need_positive) < 0) return -1; - return deleteFromPlaylist(fd, song); + + result = deleteFromPlaylist(song); + return print_playlist_result(fd, result); } static int handleDeleteId(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { int id; + enum playlist_result result; if (check_int(fd, &id, argv[1], need_positive) < 0) return -1; - return deleteFromPlaylistById(fd, id); + + result = deleteFromPlaylistById(id); + return print_playlist_result(fd, result); } static int handlePlaylist(int fd, mpd_unused int *permission, @@ -439,10 +503,10 @@ static int handlePlaylist(int fd, mpd_unused int *permission, return 0; } -static int handleShuffle(int fd, mpd_unused int *permission, +static int handleShuffle(mpd_unused int fd, mpd_unused int *permission, mpd_unused int argc, mpd_unused char *argv[]) { - shufflePlaylist(fd); + shufflePlaylist(); return 0; } @@ -456,7 +520,10 @@ static int handleClear(mpd_unused int fd, mpd_unused int *permission, static int handleSave(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - return savePlaylist(fd, argv[1]); + enum playlist_result result; + + result = savePlaylist(argv[1]); + return print_playlist_result(fd, result); } static int handleLoad(int fd, mpd_unused int *permission, @@ -497,7 +564,10 @@ static int handleLsInfo(int fd, mpd_unused int *permission, static int handleRm(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - return deletePlaylist(fd, argv[1]); + enum playlist_result result; + + result = deletePlaylist(argv[1]); + return print_playlist_result(fd, result); } static int handleRename(int fd, mpd_unused int *permission, @@ -530,20 +600,26 @@ static int handlePlaylistInfo(int fd, mpd_unused int *permission, int argc, char *argv[]) { int song = -1; + enum playlist_result result; if (argc == 2 && check_int(fd, &song, argv[1], need_positive) < 0) return -1; - return playlistInfo(fd, song); + + result = playlistInfo(fd, song); + return print_playlist_result(fd, result); } static int handlePlaylistId(int fd, mpd_unused int *permission, int argc, char *argv[]) { int id = -1; + enum playlist_result result; if (argc == 2 && check_int(fd, &id, argv[1], need_positive) < 0) return -1; - return playlistId(fd, id); + + result = playlistId(fd, id); + return print_playlist_result(fd, result); } static int handleFind(int fd, mpd_unused int *permission, @@ -869,72 +945,86 @@ static int handleMove(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { int from, to; + enum playlist_result result; if (check_int(fd, &from, argv[1], check_integer, argv[1]) < 0) return -1; if (check_int(fd, &to, argv[2], check_integer, argv[2]) < 0) return -1; - return moveSongInPlaylist(fd, from, to); + result = moveSongInPlaylist(from, to); + return print_playlist_result(fd, result); } static int handleMoveId(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { int id, to; + enum playlist_result result; if (check_int(fd, &id, argv[1], check_integer, argv[1]) < 0) return -1; if (check_int(fd, &to, argv[2], check_integer, argv[2]) < 0) return -1; - return moveSongInPlaylistById(fd, id, to); + result = moveSongInPlaylistById(id, to); + return print_playlist_result(fd, result); } static int handleSwap(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { int song1, song2; + enum playlist_result result; if (check_int(fd, &song1, argv[1], check_integer, argv[1]) < 0) return -1; if (check_int(fd, &song2, argv[2], check_integer, argv[2]) < 0) return -1; - return swapSongsInPlaylist(fd, song1, song2); + result = swapSongsInPlaylist(song1, song2); + return print_playlist_result(fd, result); } static int handleSwapId(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { int id1, id2; + enum playlist_result result; if (check_int(fd, &id1, argv[1], check_integer, argv[1]) < 0) return -1; if (check_int(fd, &id2, argv[2], check_integer, argv[2]) < 0) return -1; - return swapSongsInPlaylistById(fd, id1, id2); + result = swapSongsInPlaylistById(id1, id2); + return print_playlist_result(fd, result); } static int handleSeek(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { int song, seek_time; + enum playlist_result result; if (check_int(fd, &song, argv[1], check_integer, argv[1]) < 0) return -1; if (check_int(fd, &seek_time, argv[2], check_integer, argv[2]) < 0) return -1; - return seekSongInPlaylist(fd, song, seek_time); + + result = seekSongInPlaylist(song, seek_time); + return print_playlist_result(fd, result); } static int handleSeekId(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { int id, seek_time; + enum playlist_result result; if (check_int(fd, &id, argv[1], check_integer, argv[1]) < 0) return -1; if (check_int(fd, &seek_time, argv[2], check_integer, argv[2]) < 0) return -1; - return seekSongInPlaylistById(fd, id, seek_time); + + result = seekSongInPlaylistById(id, seek_time); + return print_playlist_result(fd, result); } static int handleListAllInfo(int fd, mpd_unused int *permission, diff --git a/src/dbUtils.c b/src/dbUtils.c index 8c499ec7e..ff184dc49 100644 --- a/src/dbUtils.c +++ b/src/dbUtils.c @@ -173,10 +173,10 @@ int printAllIn(int fd, const char *name) printDirectoryInDirectory, NULL); } -static int directoryAddSongToPlaylist(int fd, Song * song, +static int directoryAddSongToPlaylist(mpd_unused int fd, Song * song, mpd_unused void *data) { - return addSongToPlaylist(fd, song, NULL); + return addSongToPlaylist(song, NULL); } static int directoryAddSongToStoredPlaylist(int fd, Song *song, void *data) diff --git a/src/playlist.c b/src/playlist.c index 7d782c1a1..9271bb037 100644 --- a/src/playlist.c +++ b/src/playlist.c @@ -301,13 +301,12 @@ static void loadPlaylistFromStateFile(FILE *fp, char *buffer, song = atoi(temp); if (!(temp = strtok(NULL, ""))) state_file_fatal(); - if (!addToPlaylist(STDERR_FILENO, temp, NULL) + if (addToPlaylist(temp, NULL) == PLAYLIST_RESULT_SUCCESS && current == song) { if (state == OB_STATE_PAUSE) ob_trigger_action(OB_ACTION_PAUSE_SET); if (state != OB_STATE_STOP) { - seekSongInPlaylist(STDERR_FILENO, - playlist.length - 1, + seekSongInPlaylist(playlist.length - 1, seek_time); } } @@ -428,7 +427,7 @@ int playlistChangesPosId(int fd, mpd_uint32 version) return 0; } -int playlistInfo(int fd, int song) +enum playlist_result playlistInfo(int fd, int song) { int i; int begin = 0; @@ -438,36 +437,31 @@ int playlistInfo(int fd, int song) begin = song; end = song + 1; } - if (song >= playlist.length) { - commandError(fd, ACK_ERROR_NO_EXIST, - "song doesn't exist: \"%i\"", song); - return -1; - } + if (song >= playlist.length) + return PLAYLIST_RESULT_BAD_RANGE; for (i = begin; i < end; i++) printPlaylistSongInfo(fd, i); - return 0; + return PLAYLIST_RESULT_SUCCESS; } -# define checkSongId(id) { \ - if(id < 0 || id >= PLAYLIST_HASH_MULT*playlist_max_length || \ - playlist.idToPosition[id] == -1 ) \ - { \ - commandError(fd, ACK_ERROR_NO_EXIST, \ - "song id doesn't exist: \"%i\"", id); \ - return -1; \ - } \ +static int song_id_exists(int id) +{ + return id >= 0 && id < PLAYLIST_HASH_MULT*playlist_max_length && + playlist.idToPosition[id] != -1; } -int playlistId(int fd, int id) +enum playlist_result playlistId(int fd, int id) { int i; int begin = 0; int end = playlist.length; if (id >= 0) { - checkSongId(id); + if (!song_id_exists(id)) + return PLAYLIST_RESULT_NO_SUCH_SONG; + begin = playlist.idToPosition[id]; end = begin + 1; } @@ -475,7 +469,7 @@ int playlistId(int fd, int id) for (i = begin; i < end; i++) printPlaylistSongInfo(fd, i); - return 0; + return PLAYLIST_RESULT_SUCCESS; } static void swapSongs(int song1, int song2) @@ -595,7 +589,7 @@ static int clear_queue(void) return playlist.queued; } -int addToPlaylist(int fd, const char *url, int *added_id) +enum playlist_result addToPlaylist(const char *url, int *added_id) { Song *song; @@ -604,13 +598,10 @@ int addToPlaylist(int fd, const char *url, int *added_id) if ((song = getSongFromDB(url))) { } else if (!(isValidRemoteUtf8Url(url) && (song = newSong(url, SONG_TYPE_URL, NULL)))) { - commandError(fd, ACK_ERROR_NO_EXIST, - "\"%s\" is not in the music db or is " - "not a valid url", url); - return -1; + return PLAYLIST_RESULT_NO_SUCH_SONG; } - return addSongToPlaylist(fd, song, added_id); + return addSongToPlaylist(song, added_id); } int addToStoredPlaylist(int fd, const char *url, const char *utf8file) @@ -641,15 +632,12 @@ fail: return -1; } -int addSongToPlaylist(int fd, Song * song, int *added_id) +enum playlist_result addSongToPlaylist(Song * song, int *added_id) { int id; - if (playlist.length == playlist_max_length) { - commandError(fd, ACK_ERROR_PLAYLIST_MAX, - "playlist is at the max size"); - return -1; - } + if (playlist.length == playlist_max_length) + return PLAYLIST_RESULT_TOO_LARGE; if (playlist_state == PLAYLIST_STATE_PLAY) { if (playlist.queued >= 0 @@ -692,24 +680,17 @@ int addSongToPlaylist(int fd, Song * song, int *added_id) if (added_id) *added_id = id; - return 0; + return PLAYLIST_RESULT_SUCCESS; } -int swapSongsInPlaylist(int fd, int song1, int song2) +enum playlist_result swapSongsInPlaylist(int song1, int song2) { int queuedSong = -1; int currentSong; - if (song1 < 0 || song1 >= playlist.length) { - commandError(fd, ACK_ERROR_NO_EXIST, - "song doesn't exist: \"%i\"", song1); - return -1; - } - if (song2 < 0 || song2 >= playlist.length) { - commandError(fd, ACK_ERROR_NO_EXIST, - "song doesn't exist: \"%i\"", song2); - return -1; - } + if (song1 < 0 || song1 >= playlist.length || + song2 < 0 || song2 >= playlist.length) + return PLAYLIST_RESULT_BAD_RANGE; if (playlist_state == PLAYLIST_STATE_PLAY) { if (playlist.queued >= 0) { @@ -747,15 +728,15 @@ int swapSongsInPlaylist(int fd, int song1, int song2) incrPlaylistVersion(); - return 0; + return PLAYLIST_RESULT_SUCCESS; } -int swapSongsInPlaylistById(int fd, int id1, int id2) +enum playlist_result swapSongsInPlaylistById(int id1, int id2) { - checkSongId(id1); - checkSongId(id2); + if (!song_id_exists(id1) || !song_id_exists(id2)) + return PLAYLIST_RESULT_NO_SUCH_SONG; - return swapSongsInPlaylist(fd, playlist.idToPosition[id1], + return swapSongsInPlaylist(playlist.idToPosition[id1], playlist.idToPosition[id2]); } @@ -766,18 +747,15 @@ int swapSongsInPlaylistById(int fd, int id1, int id2) playlist.songMod[to] = playlist.version; \ } -int deleteFromPlaylist(int fd, int song) +enum playlist_result deleteFromPlaylist(int song) { int i; int songOrder; int stop_current = 0; int prev_queued = playlist.queued; - if (song < 0 || song >= playlist.length) { - commandError(fd, ACK_ERROR_NO_EXIST, - "song doesn't exist: \"%i\"", song); - return -1; - } + if (song < 0 || song >= playlist.length) + return PLAYLIST_RESULT_BAD_RANGE; if (playlist_state == PLAYLIST_STATE_PLAY) { if (prev_queued >= 0 @@ -842,14 +820,15 @@ int deleteFromPlaylist(int fd, int song) queueNextSongInPlaylist(); } - return 0; + return PLAYLIST_RESULT_SUCCESS; } -int deleteFromPlaylistById(int fd, int id) +enum playlist_result deleteFromPlaylistById(int id) { - checkSongId(id); + if (!song_id_exists(id)) + return PLAYLIST_RESULT_NO_SUCH_SONG; - return deleteFromPlaylist(fd, playlist.idToPosition[id]); + return deleteFromPlaylist(playlist.idToPosition[id]); } void deleteASongFromPlaylist(Song * song) @@ -861,7 +840,7 @@ void deleteASongFromPlaylist(Song * song) for (i = 0; i < playlist.length; i++) { if (song == playlist.songs[i]) { - deleteFromPlaylist(STDERR_FILENO, i); + deleteFromPlaylist(i); } } } @@ -907,7 +886,7 @@ static void play_order_num(int order_num, float seek_time) playlist.current = order_num; } -int playPlaylist(int fd, int song, int stopOnError) +enum playlist_result playPlaylist(int song, int stopOnError) { int i = song; @@ -917,11 +896,11 @@ int playPlaylist(int fd, int song, int stopOnError) if (song == -1) { if (playlist.length == 0) - return 0; + return PLAYLIST_RESULT_SUCCESS; if (playlist_state == PLAYLIST_STATE_PLAY) { ob_trigger_action(OB_ACTION_PAUSE_UNSET); - return 0; + return PLAYLIST_RESULT_SUCCESS; } if (playlist.current >= 0 && playlist.current < playlist.length) { i = playlist.current; @@ -929,9 +908,7 @@ int playPlaylist(int fd, int song, int stopOnError) i = 0; } } else if (song < 0 || song >= playlist.length) { - commandError(fd, ACK_ERROR_NO_EXIST, - "song doesn't exist: \"%i\"", song); - return -1; + return PLAYLIST_RESULT_BAD_RANGE; } if (playlist.random) { @@ -953,18 +930,19 @@ int playPlaylist(int fd, int song, int stopOnError) ERROR(__FILE__ ": %d current:%d\n", __LINE__, playlist.current); ob_trigger_action(OB_ACTION_PAUSE_UNSET); play_order_num(i, 0); - return 0; + return PLAYLIST_RESULT_SUCCESS; } -int playPlaylistById(int fd, int id, int stopOnError) +enum playlist_result playPlaylistById(int id, int stopOnError) { if (id == -1) { - return playPlaylist(fd, id, stopOnError); + return playPlaylist(id, stopOnError); } - checkSongId(id); + if (!song_id_exists(id)) + return PLAYLIST_RESULT_NO_SUCH_SONG; - return playPlaylist(fd, playlist.idToPosition[id], stopOnError); + return playPlaylist(playlist.idToPosition[id], stopOnError); } /* This is used when we stream data out to shout while playing static files */ @@ -1051,7 +1029,7 @@ void setPlaylistRepeatStatus(int status) playlist.repeat = status; } -int moveSongInPlaylist(int fd, int from, int to) +enum playlist_result moveSongInPlaylist(int from, int to) { int i; Song *tmpSong; @@ -1059,21 +1037,15 @@ int moveSongInPlaylist(int fd, int from, int to) int currentSong; int queued_is_current = (playlist.queued == playlist.current); - if (from < 0 || from >= playlist.length) { - commandError(fd, ACK_ERROR_NO_EXIST, - "song doesn't exist: \"%i\"", from); - return -1; - } + if (from < 0 || from >= playlist.length) + return PLAYLIST_RESULT_BAD_RANGE; if ((to >= 0 && to >= playlist.length) || - (to < 0 && abs(to) > playlist.length)) { - commandError(fd, ACK_ERROR_NO_EXIST, - "song doesn't exist: \"%i\"", to); - return -1; - } + (to < 0 && abs(to) > playlist.length)) + return PLAYLIST_RESULT_BAD_RANGE; if (from == to) /* no-op */ - return 0; + return PLAYLIST_RESULT_SUCCESS; /* * (to < 0) => move to offset from current song @@ -1083,7 +1055,7 @@ int moveSongInPlaylist(int fd, int from, int to) if (to < 0 && playlist.current >= 0) { if (currentSong == from) /* no-op, can't be moved to offset of itself */ - return 0; + return PLAYLIST_RESULT_SUCCESS; to = (currentSong + abs(to)) % playlist.length; } @@ -1138,14 +1110,15 @@ int moveSongInPlaylist(int fd, int from, int to) queueNextSongInPlaylist(); incrPlaylistVersion(); - return 0; + return PLAYLIST_RESULT_SUCCESS; } -int moveSongInPlaylistById(int fd, int id1, int to) +enum playlist_result moveSongInPlaylistById(int id1, int to) { - checkSongId(id1); + if (!song_id_exists(id1)) + return PLAYLIST_RESULT_NO_SUCH_SONG; - return moveSongInPlaylist(fd, playlist.idToPosition[id1], to); + return moveSongInPlaylist(playlist.idToPosition[id1], to); } static void orderPlaylist(void) @@ -1254,7 +1227,7 @@ void previousSongInPlaylist(void) play_order_num(prev_order_num, 0); } -void shufflePlaylist(mpd_unused int fd) +void shufflePlaylist(void) { int i; int ri; @@ -1293,50 +1266,39 @@ void shufflePlaylist(mpd_unused int fd) } } -int deletePlaylist(int fd, const char *utf8file) +enum playlist_result deletePlaylist(const char *utf8file) { char path_max_tmp[MPD_PATH_MAX]; utf8_to_fs_playlist_path(path_max_tmp, utf8file); - if (!isPlaylist(path_max_tmp)) { - commandError(fd, ACK_ERROR_NO_EXIST, - "playlist \"%s\" not found", utf8file); - return -1; - } + if (!isPlaylist(path_max_tmp)) + return PLAYLIST_RESULT_NO_SUCH_LIST; - if (unlink(path_max_tmp) < 0) { - commandError(fd, ACK_ERROR_SYSTEM, - "problems deleting file"); - return -1; - } + if (unlink(path_max_tmp) < 0) + return PLAYLIST_RESULT_ERRNO; - return 0; + return PLAYLIST_RESULT_SUCCESS; } -int savePlaylist(int fd, const char *utf8file) +enum playlist_result savePlaylist(const char *utf8file) { FILE *fp; int i; struct stat sb; char path_max_tmp[MPD_PATH_MAX]; - if (!valid_playlist_name(fd, utf8file)) - return -1; + if (!is_valid_playlist_name(utf8file)) + return PLAYLIST_RESULT_BAD_NAME; utf8_to_fs_playlist_path(path_max_tmp, utf8file); - if (!stat(path_max_tmp, &sb)) { - commandError(fd, ACK_ERROR_EXIST, "a file or directory already " - "exists with the name \"%s\"", utf8file); - return -1; - } + if (!stat(path_max_tmp, &sb)) + return PLAYLIST_RESULT_LIST_EXISTS; while (!(fp = fopen(path_max_tmp, "w")) && errno == EINTR); - if (fp == NULL) { - commandError(fd, ACK_ERROR_SYSTEM, "failed to create file"); - return -1; - } + if (fp == NULL) + return PLAYLIST_RESULT_ERRNO; for (i = 0; i < playlist.length; i++) { char tmp[MPD_PATH_MAX]; @@ -1353,7 +1315,7 @@ int savePlaylist(int fd, const char *utf8file) while (fclose(fp) && errno == EINTR) ; - return 0; + return PLAYLIST_RESULT_SUCCESS; } int getPlaylistCurrentSong(void) @@ -1380,16 +1342,13 @@ int getPlaylistLength(void) * This command will always return 0 regardless of whether or * not the seek succeeded (it's always been the case, apparently) */ -int seekSongInPlaylist(int fd, int song, float seek_time) +enum playlist_result seekSongInPlaylist(int song, float seek_time) { int i = song; char path[MPD_PATH_MAX]; - if (song < 0 || song >= playlist.length) { - commandError(fd, ACK_ERROR_NO_EXIST, - "song doesn't exist: \"%i\"", song); - return -1; - } + if (song < 0 || song >= playlist.length) + return PLAYLIST_RESULT_BAD_RANGE; if (playlist.random) for (i = 0; song != playlist.order[i]; i++) ; @@ -1402,7 +1361,7 @@ int seekSongInPlaylist(int fd, int song, float seek_time) (playlist.current == i && playlist.queued == i)) { dc_trigger_action(DC_ACTION_SEEK, seek_time); if (dc.seek_where != DC_SEEK_MISMATCH) - return 0; + return PLAYLIST_RESULT_SUCCESS; /* * if near end of decoding can cause seek to fail (since we're * already on another song) (leading to DC_SEEK_MISMATCH), @@ -1412,14 +1371,15 @@ int seekSongInPlaylist(int fd, int song, float seek_time) DEBUG("playlist: seek %i:\"%s\"\n", i, get_song_url(path, song_at(i))); play_order_num(i, seek_time); - return 0; + return PLAYLIST_RESULT_SUCCESS; } -int seekSongInPlaylistById(int fd, int id, float seek_time) +enum playlist_result seekSongInPlaylistById(int id, float seek_time) { - checkSongId(id); + if (!song_id_exists(id)) + return PLAYLIST_RESULT_NO_SUCH_SONG; - return seekSongInPlaylist(fd, playlist.idToPosition[id], seek_time); + return seekSongInPlaylist(playlist.idToPosition[id], seek_time); } int getPlaylistSongId(int song) @@ -1470,7 +1430,7 @@ int loadPlaylist(int fd, const char *utf8file) node = list->firstNode; while (node != NULL) { char *temp = node->data; - if ((addToPlaylist(STDERR_FILENO, temp, NULL)) < 0) { + if ((addToPlaylist(temp, NULL)) != PLAYLIST_RESULT_SUCCESS) { /* for windows compatibility, convert slashes */ char *temp2 = xstrdup(temp); char *p = temp2; @@ -1479,7 +1439,7 @@ int loadPlaylist(int fd, const char *utf8file) *p = '/'; p++; } - if ((addToPlaylist(STDERR_FILENO, temp2, NULL)) < 0) { + if ((addToPlaylist(temp, NULL)) != PLAYLIST_RESULT_SUCCESS) { commandError(fd, ACK_ERROR_PLAYLIST_LOAD, "can't add file \"%s\"", temp2); } diff --git a/src/playlist.h b/src/playlist.h index 3afe33da0..11ec63e9e 100644 --- a/src/playlist.h +++ b/src/playlist.h @@ -24,6 +24,18 @@ #define PLAYLIST_FILE_SUFFIX "m3u" #define PLAYLIST_COMMENT '#' +enum playlist_result { + PLAYLIST_RESULT_SUCCESS, + PLAYLIST_RESULT_ERRNO, + PLAYLIST_RESULT_NO_SUCH_SONG, + PLAYLIST_RESULT_NO_SUCH_LIST, + PLAYLIST_RESULT_LIST_EXISTS, + PLAYLIST_RESULT_BAD_NAME, + PLAYLIST_RESULT_BAD_RANGE, + PLAYLIST_RESULT_NOT_PLAYING, + PLAYLIST_RESULT_TOO_LARGE +}; + extern int playlist_saveAbsolutePaths; extern int playlist_max_length; @@ -40,21 +52,21 @@ void clearPlaylist(void); int clearStoredPlaylist(int fd, const char *utf8file); -int addToPlaylist(int fd, const char *file, int *added_id); +enum playlist_result addToPlaylist(const char *file, int *added_id); int addToStoredPlaylist(int fd, const char *file, const char *utf8file); -int addSongToPlaylist(int fd, Song * song, int *added_id); +enum playlist_result addSongToPlaylist(Song * song, int *added_id); void showPlaylist(int fd); -int deleteFromPlaylist(int fd, int song); +enum playlist_result deleteFromPlaylist(int song); -int deleteFromPlaylistById(int fd, int song); +enum playlist_result deleteFromPlaylistById(int song); -int playlistInfo(int fd, int song); +enum playlist_result playlistInfo(int fd, int song); -int playlistId(int fd, int song); +enum playlist_result playlistId(int fd, int song); Song *playlist_queued_song(void); @@ -64,9 +76,9 @@ int playlist_playing(void); void stopPlaylist(void); -int playPlaylist(int fd, int song, int stopOnError); +enum playlist_result playPlaylist(int song, int stopOnError); -int playPlaylistById(int fd, int song, int stopOnError); +enum playlist_result playPlaylistById(int song, int stopOnError); void nextSongInPlaylist(void); @@ -74,23 +86,21 @@ void syncPlayerAndPlaylist(void); void previousSongInPlaylist(void); -void shufflePlaylist(int fd); - -int savePlaylist(int fd, const char *utf8file); +void shufflePlaylist(void); -int deletePlaylist(int fd, const char *utf8file); +enum playlist_result savePlaylist(const char *utf8file); -int deletePlaylistById(int fd, const char *utf8file); +enum playlist_result deletePlaylist(const char *utf8file); void deleteASongFromPlaylist(Song * song); -int moveSongInPlaylist(int fd, int from, int to); +enum playlist_result moveSongInPlaylist(int from, int to); -int moveSongInPlaylistById(int fd, int id, int to); +enum playlist_result moveSongInPlaylistById(int id, int to); -int swapSongsInPlaylist(int fd, int song1, int song2); +enum playlist_result swapSongsInPlaylist(int song1, int song2); -int swapSongsInPlaylistById(int fd, int id1, int id2); +enum playlist_result swapSongsInPlaylistById(int id1, int id2); int loadPlaylist(int fd, const char *utf8file); @@ -110,9 +120,9 @@ int getPlaylistLength(void); unsigned long getPlaylistVersion(void); -int seekSongInPlaylist(int fd, int song, float seek_time); +enum playlist_result seekSongInPlaylist(int song, float seek_time); -int seekSongInPlaylistById(int fd, int id, float seek_time); +enum playlist_result seekSongInPlaylistById(int id, float seek_time); void playlistVersionChange(void); -- cgit v1.2.3 From 38526852f3c40a6014f86b832b37d31507f6131b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:44:12 +0200 Subject: playlist: don't pass "fd" to storedPlaylist.c functions Return an "enum playlist_result" value instead of calling commandError() in storedPlaylist.c. --- src/command.c | 31 +++++++--- src/dbUtils.c | 7 +-- src/playlist.c | 48 ++++++--------- src/playlist.h | 8 +-- src/storedPlaylist.c | 163 +++++++++++++++++++++------------------------------ src/storedPlaylist.h | 19 ++++-- 6 files changed, 126 insertions(+), 150 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index a4d785126..1634394d0 100644 --- a/src/command.c +++ b/src/command.c @@ -529,7 +529,10 @@ static int handleSave(int fd, mpd_unused int *permission, static int handleLoad(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - return loadPlaylist(fd, argv[1]); + enum playlist_result result; + + result = loadPlaylist(fd, argv[1]); + return print_playlist_result(fd, result); } static int handleListPlaylist(int fd, mpd_unused int *permission, @@ -573,7 +576,10 @@ static int handleRm(int fd, mpd_unused int *permission, static int handleRename(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - return renameStoredPlaylist(fd, argv[1], argv[2]); + enum playlist_result result; + + result = renameStoredPlaylist(argv[1], argv[2]); + return print_playlist_result(fd, result); } static int handlePlaylistChanges(int fd, mpd_unused int *permission, @@ -732,11 +738,13 @@ static int handlePlaylistDelete(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { char *playlist = argv[1]; int from; + enum playlist_result result; if (check_int(fd, &from, argv[2], check_integer, argv[2]) < 0) return -1; - return removeOneSongFromStoredPlaylistByPath(fd, playlist, from); + result = removeOneSongFromStoredPlaylistByPath(playlist, from); + return print_playlist_result(fd, result); } static int handlePlaylistMove(int fd, mpd_unused int *permission, @@ -744,13 +752,15 @@ static int handlePlaylistMove(int fd, mpd_unused int *permission, { char *playlist = argv[1]; int from, to; + enum playlist_result result; if (check_int(fd, &from, argv[2], check_integer, argv[2]) < 0) return -1; if (check_int(fd, &to, argv[3], check_integer, argv[3]) < 0) return -1; - return moveSongInStoredPlaylistByPath(fd, playlist, from, to); + result = moveSongInStoredPlaylistByPath(playlist, from, to); + return print_playlist_result(fd, result); } static int listHandleUpdate(int fd, @@ -1135,7 +1145,10 @@ static int handleNotcommands(int fd, mpd_unused int *permission, static int handlePlaylistClear(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - return clearStoredPlaylist(fd, argv[1]); + enum playlist_result result; + + result = clearStoredPlaylist(argv[1]); + return print_playlist_result(fd, result); } static int handlePlaylistAdd(int fd, mpd_unused int *permission, @@ -1143,11 +1156,13 @@ static int handlePlaylistAdd(int fd, mpd_unused int *permission, { char *playlist = argv[1]; char *path = argv[2]; + enum playlist_result result; if (isRemoteUrl(path)) - return addToStoredPlaylist(fd, path, playlist); - - return addAllInToStoredPlaylist(fd, path, playlist); + result = addToStoredPlaylist(path, playlist); + else + result = addAllInToStoredPlaylist(fd, path, playlist); + return print_playlist_result(fd, result); } void initCommands(void) diff --git a/src/dbUtils.c b/src/dbUtils.c index ff184dc49..c330f2ce2 100644 --- a/src/dbUtils.c +++ b/src/dbUtils.c @@ -179,11 +179,10 @@ static int directoryAddSongToPlaylist(mpd_unused int fd, Song * song, return addSongToPlaylist(song, NULL); } -static int directoryAddSongToStoredPlaylist(int fd, Song *song, void *data) +static int directoryAddSongToStoredPlaylist(mpd_unused int fd, Song *song, + void *data) { - if (appendSongToStoredPlaylistByPath(fd, (char *)data, song) != 0) - return -1; - return 0; + return appendSongToStoredPlaylistByPath((char *)data, song); } int addAllIn(int fd, const char *name) diff --git a/src/playlist.c b/src/playlist.c index 9271bb037..562a4e7e7 100644 --- a/src/playlist.c +++ b/src/playlist.c @@ -238,9 +238,9 @@ void clearPlaylist(void) incrPlaylistVersion(); } -int clearStoredPlaylist(int fd, const char *utf8file) +int clearStoredPlaylist(const char *utf8file) { - return removeAllFromStoredPlaylistByPath(fd, utf8file); + return removeAllFromStoredPlaylistByPath(utf8file); } void showPlaylist(int fd) @@ -604,32 +604,27 @@ enum playlist_result addToPlaylist(const char *url, int *added_id) return addSongToPlaylist(song, added_id); } -int addToStoredPlaylist(int fd, const char *url, const char *utf8file) +int addToStoredPlaylist(const char *url, const char *utf8file) { Song *song; DEBUG("add to stored playlist: %s\n", url); song = getSongFromDB(url); - if (song) { - appendSongToStoredPlaylistByPath(fd, utf8file, song); - return 0; - } + if (song) + return appendSongToStoredPlaylistByPath(utf8file, song); if (!isValidRemoteUtf8Url(url)) - goto fail; + return ACK_ERROR_NO_EXIST; song = newSong(url, SONG_TYPE_URL, NULL); if (song) { - appendSongToStoredPlaylistByPath(fd, utf8file, song); + int ret = appendSongToStoredPlaylistByPath(utf8file, song); freeJustSong(song); - return 0; + return ret; } -fail: - commandError(fd, ACK_ERROR_NO_EXIST, "\"%s\" is not in the music db" - "or is not a valid url", url); - return -1; + return ACK_ERROR_NO_EXIST; } enum playlist_result addSongToPlaylist(Song * song, int *added_id) @@ -1392,8 +1387,11 @@ int PlaylistInfo(int fd, const char *utf8file, int detail) ListNode *node; List *list; - if (!(list = loadStoredPlaylist(fd, utf8file))) + if (!(list = loadStoredPlaylist(utf8file))) { + commandError(fd, ACK_ERROR_NO_EXIST, "could not open playlist " + "\"%s\": %s", utf8file, strerror(errno)); return -1; + } node = list->firstNode; while (node != NULL) { @@ -1419,13 +1417,13 @@ int PlaylistInfo(int fd, const char *utf8file, int detail) return 0; } -int loadPlaylist(int fd, const char *utf8file) +enum playlist_result loadPlaylist(int fd, const char *utf8file) { ListNode *node; List *list; - if (!(list = loadStoredPlaylist(fd, utf8file))) - return -1; + if (!(list = loadStoredPlaylist(utf8file))) + return PLAYLIST_RESULT_NO_SUCH_LIST; node = list->firstNode; while (node != NULL) { @@ -1450,7 +1448,7 @@ int loadPlaylist(int fd, const char *utf8file) } freeList(list); - return 0; + return PLAYLIST_RESULT_SUCCESS; } void searchForSongsInPlaylist(int fd, int numItems, LocateTagItem * items) @@ -1502,18 +1500,6 @@ int is_valid_playlist_name(const char *utf8path) strchr(utf8path, '\r') == NULL; } -int valid_playlist_name(int err_fd, const char *utf8path) -{ - if (!is_valid_playlist_name(utf8path)) { - commandError(err_fd, ACK_ERROR_ARG, "playlist name \"%s\" is " - "invalid: playlist names may not contain slashes," - " newlines or carriage returns", - utf8path); - return 0; - } - return 1; -} - int playlist_playing(void) { return (playlist_state == PLAYLIST_STATE_PLAY); diff --git a/src/playlist.h b/src/playlist.h index 11ec63e9e..2d8d43ec6 100644 --- a/src/playlist.h +++ b/src/playlist.h @@ -50,11 +50,11 @@ void savePlaylistState(FILE *); void clearPlaylist(void); -int clearStoredPlaylist(int fd, const char *utf8file); +int clearStoredPlaylist(const char *utf8file); enum playlist_result addToPlaylist(const char *file, int *added_id); -int addToStoredPlaylist(int fd, const char *file, const char *utf8file); +int addToStoredPlaylist(const char *file, const char *utf8file); enum playlist_result addSongToPlaylist(Song * song, int *added_id); @@ -102,7 +102,7 @@ enum playlist_result swapSongsInPlaylist(int song1, int song2); enum playlist_result swapSongsInPlaylistById(int id1, int id2); -int loadPlaylist(int fd, const char *utf8file); +enum playlist_result loadPlaylist(int fd, const char *utf8file); int getPlaylistRepeatStatus(void); @@ -138,8 +138,6 @@ void findSongsInPlaylist(int fd, int numItems, LocateTagItem * items); int is_valid_playlist_name(const char *utf8path); -int valid_playlist_name(int err_fd, const char *utf8path); - struct mpd_tag *playlist_current_tag(void); #endif diff --git a/src/storedPlaylist.c b/src/storedPlaylist.c index ba0451f38..c1452ddb9 100644 --- a/src/storedPlaylist.c +++ b/src/storedPlaylist.c @@ -19,8 +19,6 @@ #include "storedPlaylist.h" #include "path.h" #include "utils.h" -#include "ack.h" -#include "command.h" #include "ls.h" #include "directory.h" #include "os_compat.h" @@ -60,7 +58,8 @@ static ListNode *nodeOfStoredPlaylist(List *list, int idx) return NULL; } -static int writeStoredPlaylistToPath(int fd, List *list, const char *utf8path) +static enum playlist_result +writeStoredPlaylistToPath(List *list, const char *utf8path) { ListNode *node; FILE *file; @@ -69,17 +68,14 @@ static int writeStoredPlaylistToPath(int fd, List *list, const char *utf8path) assert(utf8path); - if (!valid_playlist_name(fd, utf8path)) - return -1; + if (!is_valid_playlist_name(utf8path)) + return PLAYLIST_RESULT_BAD_NAME; utf8_to_fs_playlist_path(path_max_tmp, utf8path); while (!(file = fopen(path_max_tmp, "w")) && errno == EINTR); - if (file == NULL) { - commandError(fd, ACK_ERROR_NO_EXIST, "could not open file " - "\"%s\": %s", path_max_tmp, strerror(errno)); - return -1; - } + if (file == NULL) + return PLAYLIST_RESULT_ERRNO; node = list->firstNode; while (node != NULL) { @@ -91,10 +87,10 @@ static int writeStoredPlaylistToPath(int fd, List *list, const char *utf8path) } while (fclose(file) != 0 && errno == EINTR); - return 0; + return PLAYLIST_RESULT_SUCCESS; } -List *loadStoredPlaylist(int fd, const char *utf8path) +List *loadStoredPlaylist(const char *utf8path) { List *list; FILE *file; @@ -102,16 +98,13 @@ List *loadStoredPlaylist(int fd, const char *utf8path) char path_max_tmp[MPD_PATH_MAX]; const size_t musicDir_len = strlen(musicDir); - if (!valid_playlist_name(fd, utf8path)) + if (!is_valid_playlist_name(utf8path)) return NULL; utf8_to_fs_playlist_path(path_max_tmp, utf8path); while (!(file = fopen(path_max_tmp, "r")) && errno == EINTR); - if (file == NULL) { - commandError(fd, ACK_ERROR_NO_EXIST, "could not open file " - "\"%s\": %s", path_max_tmp, strerror(errno)); + if (file == NULL) return NULL; - } list = makeList(DEFAULT_FREE_DATA_FUNC, 0); @@ -139,15 +132,13 @@ List *loadStoredPlaylist(int fd, const char *utf8path) return list; } -static int moveSongInStoredPlaylist(int fd, List *list, int src, int dest) +static int moveSongInStoredPlaylist(List *list, int src, int dest) { ListNode *srcNode, *destNode; if (src >= list->numberOfNodes || dest >= list->numberOfNodes || - src < 0 || dest < 0 || src == dest) { - commandError(fd, ACK_ERROR_ARG, "argument out of range"); + src < 0 || dest < 0 || src == dest) return -1; - } srcNode = nodeOfStoredPlaylist(list, src); if (!srcNode) @@ -201,90 +192,78 @@ static int moveSongInStoredPlaylist(int fd, List *list, int src, int dest) return 0; } -int moveSongInStoredPlaylistByPath(int fd, const char *utf8path, - int src, int dest) +enum playlist_result +moveSongInStoredPlaylistByPath(const char *utf8path, int src, int dest) { List *list; + enum playlist_result result; - if (!(list = loadStoredPlaylist(fd, utf8path))) { - commandError(fd, ACK_ERROR_UNKNOWN, "could not open playlist"); - return -1; - } + if (!(list = loadStoredPlaylist(utf8path))) + return PLAYLIST_RESULT_NO_SUCH_LIST; - if (moveSongInStoredPlaylist(fd, list, src, dest) != 0) { + if (moveSongInStoredPlaylist(list, src, dest) != 0) { freeList(list); - return -1; + return PLAYLIST_RESULT_BAD_RANGE; } - if (writeStoredPlaylistToPath(fd, list, utf8path) != 0) { - commandError(fd, ACK_ERROR_UNKNOWN, "failed to save playlist"); - freeList(list); - return -1; - } + result = writeStoredPlaylistToPath(list, utf8path); freeList(list); - return 0; + return result; } -int removeAllFromStoredPlaylistByPath(int fd, const char *utf8path) +enum playlist_result +removeAllFromStoredPlaylistByPath(const char *utf8path) { char filename[MPD_PATH_MAX]; FILE *file; - if (!valid_playlist_name(fd, utf8path)) - return -1; + if (!is_valid_playlist_name(utf8path)) + return PLAYLIST_RESULT_BAD_NAME; + utf8_to_fs_playlist_path(filename, utf8path); while (!(file = fopen(filename, "w")) && errno == EINTR); - if (file == NULL) { - commandError(fd, ACK_ERROR_NO_EXIST, "could not open file " - "\"%s\": %s", filename, strerror(errno)); - return -1; - } + if (file == NULL) + return PLAYLIST_RESULT_ERRNO; while (fclose(file) != 0 && errno == EINTR); - return 0; + return PLAYLIST_RESULT_SUCCESS; } -static int removeOneSongFromStoredPlaylist(int fd, List *list, int pos) +static int removeOneSongFromStoredPlaylist(List *list, int pos) { ListNode *node = nodeOfStoredPlaylist(list, pos); - if (!node) { - commandError(fd, ACK_ERROR_ARG, - "could not find song at position"); + if (!node) return -1; - } deleteNodeFromList(list, node); return 0; } -int removeOneSongFromStoredPlaylistByPath(int fd, const char *utf8path, int pos) +enum playlist_result +removeOneSongFromStoredPlaylistByPath(const char *utf8path, int pos) { List *list; + enum playlist_result result; - if (!(list = loadStoredPlaylist(fd, utf8path))) { - commandError(fd, ACK_ERROR_UNKNOWN, "could not open playlist"); - return -1; - } + if (!(list = loadStoredPlaylist(utf8path))) + return PLAYLIST_RESULT_NO_SUCH_LIST; - if (removeOneSongFromStoredPlaylist(fd, list, pos) != 0) { + if (removeOneSongFromStoredPlaylist(list, pos) != 0) { freeList(list); - return -1; + return PLAYLIST_RESULT_BAD_RANGE; } - if (writeStoredPlaylistToPath(fd, list, utf8path) != 0) { - commandError(fd, ACK_ERROR_UNKNOWN, "failed to save playlist"); - freeList(list); - return -1; - } + result = writeStoredPlaylistToPath(list, utf8path); freeList(list); - return 0; + return result; } -int appendSongToStoredPlaylistByPath(int fd, const char *utf8path, Song *song) +enum playlist_result +appendSongToStoredPlaylistByPath(const char *utf8path, Song *song) { FILE *file; char *s; @@ -292,27 +271,28 @@ int appendSongToStoredPlaylistByPath(int fd, const char *utf8path, Song *song) char path_max_tmp[MPD_PATH_MAX]; char path_max_tmp2[MPD_PATH_MAX]; - if (!valid_playlist_name(fd, utf8path)) - return -1; + if (!is_valid_playlist_name(utf8path)) + return PLAYLIST_RESULT_BAD_NAME; utf8_to_fs_playlist_path(path_max_tmp, utf8path); while (!(file = fopen(path_max_tmp, "a")) && errno == EINTR); if (file == NULL) { - commandError(fd, ACK_ERROR_NO_EXIST, "could not open file " - "\"%s\": %s", path_max_tmp, strerror(errno)); - return -1; + int save_errno = errno; + while (fclose(file) != 0 && errno == EINTR); + errno = save_errno; + return PLAYLIST_RESULT_ERRNO; } + if (fstat(fileno(file), &st) < 0) { - commandError(fd, ACK_ERROR_NO_EXIST, "could not stat file " - "\"%s\": %s", path_max_tmp, strerror(errno)); + int save_errno = errno; while (fclose(file) != 0 && errno == EINTR); - return -1; + errno = save_errno; + return PLAYLIST_RESULT_ERRNO; } + if (st.st_size >= ((MPD_PATH_MAX+1) * playlist_max_length)) { while (fclose(file) != 0 && errno == EINTR); - commandError(fd, ACK_ERROR_PLAYLIST_MAX, - "playlist is at the max size"); - return -1; + return PLAYLIST_RESULT_TOO_LARGE; } s = utf8_to_fs_charset(path_max_tmp2, get_song_url(path_max_tmp, song)); @@ -323,40 +303,31 @@ int appendSongToStoredPlaylistByPath(int fd, const char *utf8path, Song *song) fprintf(file, "%s\n", s); while (fclose(file) != 0 && errno == EINTR); - return 0; + return PLAYLIST_RESULT_SUCCESS; } -int renameStoredPlaylist(int fd, const char *utf8from, const char *utf8to) +enum playlist_result +renameStoredPlaylist(const char *utf8from, const char *utf8to) { struct stat st; char from[MPD_PATH_MAX]; char to[MPD_PATH_MAX]; - if (!valid_playlist_name(fd, utf8from) || - !valid_playlist_name(fd, utf8to)) - return -1; + if (!is_valid_playlist_name(utf8from) || + !is_valid_playlist_name(utf8to)) + return PLAYLIST_RESULT_BAD_NAME; utf8_to_fs_playlist_path(from, utf8from); utf8_to_fs_playlist_path(to, utf8to); - if (stat(from, &st) != 0) { - commandError(fd, ACK_ERROR_NO_EXIST, - "no playlist named \"%s\"", utf8from); - return -1; - } + if (stat(from, &st) != 0) + return PLAYLIST_RESULT_NO_SUCH_LIST; - if (stat(to, &st) == 0) { - commandError(fd, ACK_ERROR_EXIST, "a file or directory " - "already exists with the name \"%s\"", utf8to); - return -1; - } + if (stat(to, &st) == 0) + return PLAYLIST_RESULT_LIST_EXISTS; - if (rename(from, to) < 0) { - commandError(fd, ACK_ERROR_UNKNOWN, - "could not rename playlist \"%s\" to \"%s\": %s", - utf8from, utf8to, strerror(errno)); - return -1; - } + if (rename(from, to) < 0) + return PLAYLIST_RESULT_ERRNO; - return 0; + return PLAYLIST_RESULT_SUCCESS; } diff --git a/src/storedPlaylist.h b/src/storedPlaylist.h index a7806386b..964669d35 100644 --- a/src/storedPlaylist.h +++ b/src/storedPlaylist.h @@ -23,14 +23,21 @@ #include "list.h" #include "playlist.h" -List *loadStoredPlaylist(int fd, const char *utf8path); +List *loadStoredPlaylist(const char *utf8path); -int moveSongInStoredPlaylistByPath(int fd, const char *utf8path, int src, int dest); -int removeAllFromStoredPlaylistByPath(int fd, const char *utf8path); -int removeOneSongFromStoredPlaylistByPath(int fd, const char *utf8path, int pos); +enum playlist_result +moveSongInStoredPlaylistByPath(const char *utf8path, int src, int dest); -int appendSongToStoredPlaylistByPath(int fd, const char *utf8path, Song *song); +enum playlist_result +removeAllFromStoredPlaylistByPath(const char *utf8path); -int renameStoredPlaylist(int fd, const char *utf8from, const char *utf8to); +enum playlist_result +removeOneSongFromStoredPlaylistByPath(const char *utf8path, int pos); + +enum playlist_result +appendSongToStoredPlaylistByPath(const char *utf8path, Song *song); + +enum playlist_result +renameStoredPlaylist(const char *utf8from, const char *utf8to); #endif -- cgit v1.2.3 From 3884c89a2ec5378f54ebbaf4402d9d5438e80b92 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:44:20 +0200 Subject: playlist: PlaylistInfo() does not call commandError() Continuing the effort of removing protocol specific calls from the core libraries: let the command.c code call commandError() based on PlaylistInfo's return value. --- src/command.c | 16 ++++++++++++++-- src/playlist.c | 5 +---- 2 files changed, 15 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index 1634394d0..59a99141a 100644 --- a/src/command.c +++ b/src/command.c @@ -538,13 +538,25 @@ static int handleLoad(int fd, mpd_unused int *permission, static int handleListPlaylist(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - return PlaylistInfo(fd, argv[1], 0); + int ret; + + ret = PlaylistInfo(fd, argv[1], 0); + if (ret == -1) + commandError(fd, ACK_ERROR_NO_EXIST, "No such playlist"); + + return ret; } static int handleListPlaylistInfo(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - return PlaylistInfo(fd, argv[1], 1); + int ret; + + ret = PlaylistInfo(fd, argv[1], 1); + if (ret == -1) + commandError(fd, ACK_ERROR_NO_EXIST, "No such playlist"); + + return ret; } static int handleLsInfo(int fd, mpd_unused int *permission, diff --git a/src/playlist.c b/src/playlist.c index 562a4e7e7..ca79393b0 100644 --- a/src/playlist.c +++ b/src/playlist.c @@ -1387,11 +1387,8 @@ int PlaylistInfo(int fd, const char *utf8file, int detail) ListNode *node; List *list; - if (!(list = loadStoredPlaylist(utf8file))) { - commandError(fd, ACK_ERROR_NO_EXIST, "could not open playlist " - "\"%s\": %s", utf8file, strerror(errno)); + if (!(list = loadStoredPlaylist(utf8file))) return -1; - } node = list->firstNode; while (node != NULL) { -- cgit v1.2.3 From dcc575a3a877e342e895df9fc99108028151cc6a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:48:24 +0200 Subject: directory: don't pass fd to traverseAllIn() callbacks Database traversal should be generic, and not bound to a client connection. This is the first step: no file descriptor for the callback functions forEachSong() and forEachDir(). If a callback needs the file descriptor, it has to be passed in the void*data pointer somehow; some callbacks might need a new struct for passing more than one parameter. This might look a bit cumbersome right now, but our goal is to have a clean API. --- src/dbUtils.c | 103 ++++++++++++++++++++++++++++++++++++------------------- src/directory.c | 23 ++++++------- src/directory.h | 4 +-- src/tagTracker.c | 2 +- 4 files changed, 81 insertions(+), 51 deletions(-) (limited to 'src') diff --git a/src/dbUtils.c b/src/dbUtils.c index c330f2ce2..efce0f4aa 100644 --- a/src/dbUtils.c +++ b/src/dbUtils.c @@ -45,7 +45,7 @@ typedef struct _SearchStats { unsigned long playTime; } SearchStats; -static int countSongsInDirectory(mpd_unused int fd, Directory * directory, +static int countSongsInDirectory(Directory * directory, void *data) { int *count = (int *)data; @@ -55,24 +55,32 @@ static int countSongsInDirectory(mpd_unused int fd, Directory * directory, return 0; } -static int printDirectoryInDirectory(int fd, Directory * directory, - mpd_unused void *data) +static int printDirectoryInDirectory(Directory * directory, void *data) { + int fd = (int)(size_t)data; if (directory->path) { fdprintf(fd, "directory: %s\n", getDirectoryPath(directory)); } return 0; } -static int printSongInDirectory(int fd, Song * song, mpd_unused void *data) +static int printSongInDirectory(Song * song, mpd_unused void *data) { + int fd = (int)(size_t)data; printSongUrl(fd, song); return 0; } -static int searchInDirectory(int fd, Song * song, void *data) +struct search_data { + int fd; + LocateTagItemArray array; +}; + +static int searchInDirectory(Song * song, void *_data) { - LocateTagItemArray *array = data; + struct search_data *data = _data; + int fd = data->fd; + LocateTagItemArray *array = &data->array; if (strstrSearchTags(song, array->numItems, array->items)) printSongInfo(fd, song); @@ -87,17 +95,18 @@ int searchForSongsIn(int fd, const char *name, int numItems, int i; char **originalNeedles = xmalloc(numItems * sizeof(char *)); - LocateTagItemArray array; + struct search_data data; for (i = 0; i < numItems; i++) { originalNeedles[i] = items[i].needle; items[i].needle = strDupToUpper(originalNeedles[i]); } - array.numItems = numItems; - array.items = items; + data.fd = fd; + data.array.numItems = numItems; + data.array.items = items; - ret = traverseAllIn(fd, name, searchInDirectory, NULL, &array); + ret = traverseAllIn(fd, name, searchInDirectory, NULL, &data); for (i = 0; i < numItems; i++) { free(items[i].needle); @@ -109,9 +118,11 @@ int searchForSongsIn(int fd, const char *name, int numItems, return ret; } -static int findInDirectory(int fd, Song * song, void *data) +static int findInDirectory(Song * song, void *_data) { - LocateTagItemArray *array = data; + struct search_data *data = _data; + int fd = data->fd; + LocateTagItemArray *array = &data->array; if (tagItemsFoundAndMatches(song, array->numItems, array->items)) printSongInfo(fd, song); @@ -121,12 +132,13 @@ static int findInDirectory(int fd, Song * song, void *data) int findSongsIn(int fd, const char *name, int numItems, LocateTagItem * items) { - LocateTagItemArray array; + struct search_data data; - array.numItems = numItems; - array.items = items; + data.fd = fd; + data.array.numItems = numItems; + data.array.items = items; - return traverseAllIn(fd, name, findInDirectory, NULL, (void *)&array); + return traverseAllIn(fd, name, findInDirectory, NULL, &data); } static void printSearchStats(int fd, SearchStats *stats) @@ -135,7 +147,7 @@ static void printSearchStats(int fd, SearchStats *stats) fdprintf(fd, "playtime: %li\n", stats->playTime); } -static int searchStatsInDirectory(mpd_unused int fd, Song * song, void *data) +static int searchStatsInDirectory(Song * song, void *data) { SearchStats *stats = data; @@ -170,19 +182,25 @@ int searchStatsForSongsIn(int fd, const char *name, int numItems, int printAllIn(int fd, const char *name) { return traverseAllIn(fd, name, printSongInDirectory, - printDirectoryInDirectory, NULL); + printDirectoryInDirectory, (void*)(size_t)fd); } -static int directoryAddSongToPlaylist(mpd_unused int fd, Song * song, - mpd_unused void *data) +static int directoryAddSongToPlaylist(Song * song, mpd_unused void *data) { return addSongToPlaylist(song, NULL); } -static int directoryAddSongToStoredPlaylist(mpd_unused int fd, Song *song, - void *data) +struct add_data { + const char *path; +}; + +static int directoryAddSongToStoredPlaylist(Song *song, void *_data) { - return appendSongToStoredPlaylistByPath((char *)data, song); + struct add_data *data = _data; + + if (appendSongToStoredPlaylistByPath(data->path, song) != 0) + return -1; + return 0; } int addAllIn(int fd, const char *name) @@ -192,16 +210,22 @@ int addAllIn(int fd, const char *name) int addAllInToStoredPlaylist(int fd, const char *name, const char *utf8file) { + struct add_data data = { + .path = utf8file, + }; + return traverseAllIn(fd, name, directoryAddSongToStoredPlaylist, NULL, - (void *)utf8file); + &data); } -static int directoryPrintSongInfo(int fd, Song * song, mpd_unused void *data) +static int directoryPrintSongInfo(Song * song, void *data) { + int fd = (int)(size_t)data; + return printSongInfo(fd, song); } -static int sumSongTime(mpd_unused int fd, Song * song, void *data) +static int sumSongTime(Song * song, void *data) { unsigned long *sum_time = (unsigned long *)data; @@ -214,7 +238,7 @@ static int sumSongTime(mpd_unused int fd, Song * song, void *data) int printInfoForAllIn(int fd, const char *name) { return traverseAllIn(fd, name, directoryPrintSongInfo, - printDirectoryInDirectory, NULL); + printDirectoryInDirectory, (void*)(size_t)fd); } int countSongsIn(int fd, const char *name) @@ -274,13 +298,19 @@ static void visitTag(int fd, Song * song, enum tag_type tagType) } } -static int listUniqueTagsInDirectory(int fd, Song * song, void *data) +struct list_tags_data { + int fd; + ListCommandItem *item; +}; + +static int listUniqueTagsInDirectory(Song * song, void *_data) { - ListCommandItem *item = data; + struct list_tags_data *data = _data; + ListCommandItem *item = data->item; if (tagItemsFoundAndMatches(song, item->numConditionals, item->conditionals)) { - visitTag(fd, song, item->tagType); + visitTag(data->fd, song, item->tagType); } return 0; @@ -292,13 +322,17 @@ int listAllUniqueTags(int fd, int type, int numConditionals, int ret; ListCommandItem *item = newListCommandItem(type, numConditionals, conditionals); + struct list_tags_data data = { + .fd = fd, + .item = item, + }; if (type >= 0 && type <= TAG_NUM_OF_ITEM_TYPES) { resetVisitedFlagsInTagTracker(type); } ret = traverseAllIn(fd, NULL, listUniqueTagsInDirectory, NULL, - (void *)item); + &data); if (type >= 0 && type <= TAG_NUM_OF_ITEM_TYPES) { printVisitedInTagTracker(fd, type); @@ -309,9 +343,7 @@ int listAllUniqueTags(int fd, int type, int numConditionals, return ret; } -static int sumSavedFilenameMemoryInDirectory(mpd_unused int fd, - Directory * dir, - void *data) +static int sumSavedFilenameMemoryInDirectory(Directory * dir, void *data) { int *sum = data; @@ -324,8 +356,7 @@ static int sumSavedFilenameMemoryInDirectory(mpd_unused int fd, return 0; } -static int sumSavedFilenameMemoryInSong(mpd_unused int fd, Song * song, - void *data) +static int sumSavedFilenameMemoryInSong(Song * song, void *data) { int *sum = data; diff --git a/src/directory.c b/src/directory.c index 32db02a9b..48d3249ce 100644 --- a/src/directory.c +++ b/src/directory.c @@ -1180,11 +1180,10 @@ void updateMp3Directory(void) return; } -static int traverseAllInSubDirectory(int fd, Directory * directory, - int (*forEachSong) (int, Song *, - void *), - int (*forEachDir) (int, Directory *, - void *), void *data) +static int traverseAllInSubDirectory(Directory * directory, + int (*forEachSong) (Song *, void *), + int (*forEachDir) (Directory *, void *), + void *data) { ListNode *node = directory->songs->firstNode; Song *song; @@ -1192,7 +1191,7 @@ static int traverseAllInSubDirectory(int fd, Directory * directory, int errFlag = 0; if (forEachDir) { - errFlag = forEachDir(fd, directory, data); + errFlag = forEachDir(directory, data); if (errFlag) return errFlag; } @@ -1200,7 +1199,7 @@ static int traverseAllInSubDirectory(int fd, Directory * directory, if (forEachSong) { while (node != NULL && !errFlag) { song = (Song *) node->data; - errFlag = forEachSong(fd, song, data); + errFlag = forEachSong(song, data); node = node->nextNode; } if (errFlag) @@ -1211,7 +1210,7 @@ static int traverseAllInSubDirectory(int fd, Directory * directory, while (node != NULL && !errFlag) { dir = (Directory *) node->data; - errFlag = traverseAllInSubDirectory(fd, dir, forEachSong, + errFlag = traverseAllInSubDirectory(dir, forEachSong, forEachDir, data); node = node->nextNode; } @@ -1220,22 +1219,22 @@ static int traverseAllInSubDirectory(int fd, Directory * directory, } int traverseAllIn(int fd, const char *name, - int (*forEachSong) (int, Song *, void *), - int (*forEachDir) (int, Directory *, void *), void *data) + int (*forEachSong) (Song *, void *), + int (*forEachDir) (Directory *, void *), void *data) { Directory *directory; if ((directory = getDirectory(name)) == NULL) { Song *song; if ((song = getSongFromDB(name)) && forEachSong) { - return forEachSong(fd, song, data); + return forEachSong(song, data); } commandError(fd, ACK_ERROR_NO_EXIST, "directory or file not found"); return -1; } - return traverseAllInSubDirectory(fd, directory, forEachSong, forEachDir, + return traverseAllInSubDirectory(directory, forEachSong, forEachDir, data); } diff --git a/src/directory.h b/src/directory.h index 00b902510..02a6b22d4 100644 --- a/src/directory.h +++ b/src/directory.h @@ -65,8 +65,8 @@ Song *getSongFromDB(const char *file); time_t getDbModTime(void); int traverseAllIn(int fd, const char *name, - int (*forEachSong) (int, Song *, void *), - int (*forEachDir) (int, Directory *, void *), void *data); + int (*forEachSong) (Song *, void *), + int (*forEachDir) (Directory *, void *), void *data); #define getDirectoryPath(dir) ((dir && dir->path) ? dir->path : "") diff --git a/src/tagTracker.c b/src/tagTracker.c index bea58ae4c..e470f85ea 100644 --- a/src/tagTracker.c +++ b/src/tagTracker.c @@ -38,7 +38,7 @@ struct visited { static struct visited *visited_heads[TAG_NUM_OF_ITEM_TYPES]; static unsigned num_visited[TAG_NUM_OF_ITEM_TYPES]; -static int visit_tag_items(int fd mpd_unused, Song *song, void *data) +static int visit_tag_items(Song *song, void *data) { enum tag_type type = (enum tag_type)(size_t)data; unsigned i; -- cgit v1.2.3 From 7d7b69e576500522a011627d7937e255aa7c16c7 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:48:37 +0200 Subject: directory: don't pass fd to traverseAllIn() This patch continues the work of the previous patch: don't pass a file descriptor at all to traverseAllIn(). Since this fd was only used to report "directory not found" errors, we can easily move that check to the caller. This is a great relief, since it removes the dependency on a client connection from a lot of enumeration functions. --- src/command.c | 47 +++++++++++++++++++++++++++++++++++++++++++---- src/dbUtils.c | 30 +++++++++++++++--------------- src/dbUtils.h | 8 ++++---- src/directory.c | 12 +++++------- src/directory.h | 2 +- src/tagTracker.c | 2 +- 6 files changed, 69 insertions(+), 32 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index 59a99141a..7a2981e68 100644 --- a/src/command.c +++ b/src/command.c @@ -440,7 +440,13 @@ static int handleAdd(int fd, mpd_unused int *permission, if (isRemoteUrl(path)) return addToPlaylist(path, NULL); - result = addAllIn(fd, path); + result = addAllIn(path); + if (result == (enum playlist_result)-1) { + commandError(fd, ACK_ERROR_NO_EXIST, + "directory or file not found"); + return -1; + } + return print_playlist_result(fd, result); } @@ -656,6 +662,9 @@ static int handleFind(int fd, mpd_unused int *permission, } ret = findSongsIn(fd, NULL, numItems, items); + if (ret == -1) + commandError(fd, ACK_ERROR_NO_EXIST, + "directory or file not found"); freeLocateTagItemArray(numItems, items); @@ -678,6 +687,9 @@ static int handleSearch(int fd, mpd_unused int *permission, } ret = searchForSongsIn(fd, NULL, numItems, items); + if (ret == -1) + commandError(fd, ACK_ERROR_NO_EXIST, + "directory or file not found"); freeLocateTagItemArray(numItems, items); @@ -700,6 +712,9 @@ static int handleCount(int fd, mpd_unused int *permission, } ret = searchStatsForSongsIn(fd, NULL, numItems, items); + if (ret == -1) + commandError(fd, ACK_ERROR_NO_EXIST, + "directory or file not found"); freeLocateTagItemArray(numItems, items); @@ -838,10 +853,17 @@ static int handleListAll(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { char *directory = NULL; + int ret; if (argc == 2) directory = argv[1]; - return printAllIn(fd, directory); + + ret = printAllIn(fd, directory); + if (ret == -1) + commandError(fd, ACK_ERROR_NO_EXIST, + "directory or file not found"); + + return ret; } static int handleVolume(int fd, mpd_unused int *permission, @@ -960,6 +982,10 @@ static int handleList(int fd, mpd_unused int *permission, if (conditionals) freeLocateTagItemArray(numConditionals, conditionals); + if (ret == -1) + commandError(fd, ACK_ERROR_NO_EXIST, + "directory or file not found"); + return ret; } @@ -1053,10 +1079,16 @@ static int handleListAllInfo(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { char *directory = NULL; + int ret; if (argc == 2) directory = argv[1]; - return printInfoForAllIn(fd, directory); + ret = printInfoForAllIn(fd, directory); + if (ret == -1) + commandError(fd, ACK_ERROR_NO_EXIST, + "directory or file not found"); + + return ret; } static int handlePing(mpd_unused int fd, mpd_unused int *permission, @@ -1173,7 +1205,14 @@ static int handlePlaylistAdd(int fd, mpd_unused int *permission, if (isRemoteUrl(path)) result = addToStoredPlaylist(path, playlist); else - result = addAllInToStoredPlaylist(fd, path, playlist); + result = addAllInToStoredPlaylist(path, playlist); + + if (result == (enum playlist_result)-1) { + commandError(fd, ACK_ERROR_NO_EXIST, + "directory or file not found"); + return -1; + } + return print_playlist_result(fd, result); } diff --git a/src/dbUtils.c b/src/dbUtils.c index efce0f4aa..e447bc5ad 100644 --- a/src/dbUtils.c +++ b/src/dbUtils.c @@ -106,7 +106,7 @@ int searchForSongsIn(int fd, const char *name, int numItems, data.array.numItems = numItems; data.array.items = items; - ret = traverseAllIn(fd, name, searchInDirectory, NULL, &data); + ret = traverseAllIn(name, searchInDirectory, NULL, &data); for (i = 0; i < numItems; i++) { free(items[i].needle); @@ -138,7 +138,7 @@ int findSongsIn(int fd, const char *name, int numItems, LocateTagItem * items) data.array.numItems = numItems; data.array.items = items; - return traverseAllIn(fd, name, findInDirectory, NULL, &data); + return traverseAllIn(name, findInDirectory, NULL, &data); } static void printSearchStats(int fd, SearchStats *stats) @@ -172,7 +172,7 @@ int searchStatsForSongsIn(int fd, const char *name, int numItems, stats.numberOfSongs = 0; stats.playTime = 0; - ret = traverseAllIn(fd, name, searchStatsInDirectory, NULL, &stats); + ret = traverseAllIn(name, searchStatsInDirectory, NULL, &stats); if (ret == 0) printSearchStats(fd, &stats); @@ -181,7 +181,7 @@ int searchStatsForSongsIn(int fd, const char *name, int numItems, int printAllIn(int fd, const char *name) { - return traverseAllIn(fd, name, printSongInDirectory, + return traverseAllIn(name, printSongInDirectory, printDirectoryInDirectory, (void*)(size_t)fd); } @@ -203,18 +203,18 @@ static int directoryAddSongToStoredPlaylist(Song *song, void *_data) return 0; } -int addAllIn(int fd, const char *name) +int addAllIn(const char *name) { - return traverseAllIn(fd, name, directoryAddSongToPlaylist, NULL, NULL); + return traverseAllIn(name, directoryAddSongToPlaylist, NULL, NULL); } -int addAllInToStoredPlaylist(int fd, const char *name, const char *utf8file) +int addAllInToStoredPlaylist(const char *name, const char *utf8file) { struct add_data data = { .path = utf8file, }; - return traverseAllIn(fd, name, directoryAddSongToStoredPlaylist, NULL, + return traverseAllIn(name, directoryAddSongToStoredPlaylist, NULL, &data); } @@ -237,26 +237,26 @@ static int sumSongTime(Song * song, void *data) int printInfoForAllIn(int fd, const char *name) { - return traverseAllIn(fd, name, directoryPrintSongInfo, + return traverseAllIn(name, directoryPrintSongInfo, printDirectoryInDirectory, (void*)(size_t)fd); } -int countSongsIn(int fd, const char *name) +int countSongsIn(const char *name) { int count = 0; void *ptr = (void *)&count; - traverseAllIn(fd, name, NULL, countSongsInDirectory, ptr); + traverseAllIn(name, NULL, countSongsInDirectory, ptr); return count; } -unsigned long sumSongTimesIn(int fd, const char *name) +unsigned long sumSongTimesIn(const char *name) { unsigned long dbPlayTime = 0; void *ptr = (void *)&dbPlayTime; - traverseAllIn(fd, name, sumSongTime, NULL, ptr); + traverseAllIn(name, sumSongTime, NULL, ptr); return dbPlayTime; } @@ -331,7 +331,7 @@ int listAllUniqueTags(int fd, int type, int numConditionals, resetVisitedFlagsInTagTracker(type); } - ret = traverseAllIn(fd, NULL, listUniqueTagsInDirectory, NULL, + ret = traverseAllIn(NULL, listUniqueTagsInDirectory, NULL, &data); if (type >= 0 && type <= TAG_NUM_OF_ITEM_TYPES) { @@ -369,7 +369,7 @@ void printSavedMemoryFromFilenames(void) { int sum = 0; - traverseAllIn(STDERR_FILENO, NULL, sumSavedFilenameMemoryInSong, + traverseAllIn(NULL, sumSavedFilenameMemoryInSong, sumSavedFilenameMemoryInDirectory, (void *)&sum); DEBUG("saved memory from filenames: %i\n", sum); diff --git a/src/dbUtils.h b/src/dbUtils.h index 89b69bfc3..592b62e95 100644 --- a/src/dbUtils.h +++ b/src/dbUtils.h @@ -23,9 +23,9 @@ int printAllIn(int fd, const char *name); -int addAllIn(int fd, const char *name); +int addAllIn(const char *name); -int addAllInToStoredPlaylist(int fd, const char *name, const char *utf8file); +int addAllInToStoredPlaylist(const char *name, const char *utf8file); int printInfoForAllIn(int fd, const char *name); @@ -37,9 +37,9 @@ int findSongsIn(int fd, const char *name, int numItems, LocateTagItem * items); int searchStatsForSongsIn(int fd, const char *name, int numItems, LocateTagItem * items); -int countSongsIn(int fd, const char *name); +int countSongsIn(const char *name); -unsigned long sumSongTimesIn(int fd, const char *name); +unsigned long sumSongTimesIn(const char *name); int listAllUniqueTags(int fd, int type, int numConditiionals, LocateTagItem * conditionals); diff --git a/src/directory.c b/src/directory.c index 48d3249ce..64d9492a8 100644 --- a/src/directory.c +++ b/src/directory.c @@ -1153,8 +1153,8 @@ int readDirectoryDB(void) readDirectoryInfo(fp, mp3rootDirectory); while (fclose(fp) && errno == EINTR) ; - stats.numberOfSongs = countSongsIn(STDERR_FILENO, NULL); - stats.dbPlayTime = sumSongTimesIn(STDERR_FILENO, NULL); + stats.numberOfSongs = countSongsIn(NULL); + stats.dbPlayTime = sumSongTimesIn(NULL); if (stat(dbFile, &st) == 0) directory_dbModTime = st.st_mtime; @@ -1218,7 +1218,7 @@ static int traverseAllInSubDirectory(Directory * directory, return errFlag; } -int traverseAllIn(int fd, const char *name, +int traverseAllIn(const char *name, int (*forEachSong) (Song *, void *), int (*forEachDir) (Directory *, void *), void *data) { @@ -1229,8 +1229,6 @@ int traverseAllIn(int fd, const char *name, if ((song = getSongFromDB(name)) && forEachSong) { return forEachSong(song, data); } - commandError(fd, ACK_ERROR_NO_EXIST, - "directory or file not found"); return -1; } @@ -1255,8 +1253,8 @@ void initMp3Directory(void) mp3rootDirectory = newDirectory(NULL, NULL); exploreDirectory(mp3rootDirectory); freeAllDirectoryStats(mp3rootDirectory); - stats.numberOfSongs = countSongsIn(STDERR_FILENO, NULL); - stats.dbPlayTime = sumSongTimesIn(STDERR_FILENO, NULL); + stats.numberOfSongs = countSongsIn(NULL); + stats.dbPlayTime = sumSongTimesIn(NULL); } static Song *getSongDetails(const char *file, const char **shortnameRet, diff --git a/src/directory.h b/src/directory.h index 02a6b22d4..19dada309 100644 --- a/src/directory.h +++ b/src/directory.h @@ -64,7 +64,7 @@ Song *getSongFromDB(const char *file); time_t getDbModTime(void); -int traverseAllIn(int fd, const char *name, +int traverseAllIn(const char *name, int (*forEachSong) (Song *, void *), int (*forEachDir) (Directory *, void *), void *data); diff --git a/src/tagTracker.c b/src/tagTracker.c index e470f85ea..dd9d56330 100644 --- a/src/tagTracker.c +++ b/src/tagTracker.c @@ -61,7 +61,7 @@ int getNumberOfTagItems(int type) resetVisitedFlagsInTagTracker(type); - traverseAllIn(-1, NULL, visit_tag_items, NULL, (void*)(size_t)type); + traverseAllIn(NULL, visit_tag_items, NULL, (void*)(size_t)type); ret = (int)num_visited[type]; resetVisitedFlagsInTagTracker(type); -- cgit v1.2.3 From 2210ddee97bd16424e218a3894fcaca52a4ee080 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:49:01 +0200 Subject: directory: printDirectoryInfo() does not call commandError() Move another ocurrence of error handling over to command.c. --- src/command.c | 4 +++- src/directory.c | 4 +--- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index 7a2981e68..e24c93d59 100644 --- a/src/command.c +++ b/src/command.c @@ -573,8 +573,10 @@ static int handleLsInfo(int fd, mpd_unused int *permission, if (argc == 2) path = argv[1]; - if (printDirectoryInfo(fd, path) < 0) + if (printDirectoryInfo(fd, path) < 0) { + commandError(fd, ACK_ERROR_NO_EXIST, "directory not found"); return -1; + } if (isRootDirectory(path)) return lsPlaylists(fd, path); diff --git a/src/directory.c b/src/directory.c index 64d9492a8..80385929c 100644 --- a/src/directory.c +++ b/src/directory.c @@ -848,10 +848,8 @@ int printDirectoryInfo(int fd, const char *name) { Directory *directory; - if ((directory = getDirectory(name)) == NULL) { - commandError(fd, ACK_ERROR_NO_EXIST, "directory not found"); + if ((directory = getDirectory(name)) == NULL) return -1; - } printDirectoryList(fd, directory->subDirectories); printSongInfoFromList(fd, directory->songs); -- cgit v1.2.3 From 288c051c55e741bbf9387579ea7f066ab6f754f0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:50:16 +0200 Subject: volume: don't pass "fd" to changeVolumeLevel() The "volume" library shouldn't talk to the client. Move error handling to command.c. --- src/command.c | 20 ++++++++++++++++---- src/volume.c | 25 +++++++++---------------- src/volume.h | 2 +- 3 files changed, 26 insertions(+), 21 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index e24c93d59..dc62481ff 100644 --- a/src/command.c +++ b/src/command.c @@ -871,21 +871,33 @@ static int handleListAll(int fd, mpd_unused int *permission, static int handleVolume(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - int change; + int change, ret; if (check_int(fd, &change, argv[1], need_integer) < 0) return -1; - return changeVolumeLevel(fd, change, 1); + + ret = changeVolumeLevel(change, 1); + if (ret == -1) + commandError(fd, ACK_ERROR_SYSTEM, + "problems setting volume"); + + return ret; } static int handleSetVol(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - int level; + int level, ret; if (check_int(fd, &level, argv[1], need_integer) < 0) return -1; - return changeVolumeLevel(fd, level, 0); + + ret = changeVolumeLevel(level, 0); + if (ret == -1) + commandError(fd, ACK_ERROR_SYSTEM, + "problems setting volume"); + + return ret; } static int handleRepeat(int fd, mpd_unused int *permission, diff --git a/src/volume.c b/src/volume.c index cf2b8eff8..428c3b8a1 100644 --- a/src/volume.c +++ b/src/volume.c @@ -17,12 +17,10 @@ */ #include "volume.h" -#include "command.h" #include "conf.h" #include "log.h" #include "gcc.h" #include "utils.h" -#include "ack.h" #include "os_compat.h" #include "outputBuffer.h" @@ -169,18 +167,15 @@ static int getOssVolumeLevel(void) return left; } -static int changeOssVolumeLevel(int fd, int change, int rel) +static int changeOssVolumeLevel(int change, int rel) { int current; int new; int level; if (rel) { - if ((current = getOssVolumeLevel()) < 0) { - commandError(fd, ACK_ERROR_SYSTEM, - "problem getting current volume"); + if ((current = getOssVolumeLevel()) < 0) return -1; - } new = current + change; } else { @@ -198,7 +193,6 @@ static int changeOssVolumeLevel(int fd, int change, int rel) if (ioctl(volume_ossFd, MIXER_WRITE(volume_ossControl), &level) < 0) { closeOssMixer(); - commandError(fd, ACK_ERROR_SYSTEM, "problems setting volume"); return -1; } @@ -328,7 +322,7 @@ static int getAlsaVolumeLevel(void) return ret; } -static int changeAlsaVolumeLevel(int fd, int change, int rel) +static int changeAlsaVolumeLevel(int change, int rel) { float vol; long level; @@ -361,7 +355,6 @@ static int changeAlsaVolumeLevel(int fd, int change, int rel) if ((err = snd_mixer_selem_set_playback_volume_all(volume_alsaElem, level)) < 0) { - commandError(fd, ACK_ERROR_SYSTEM, "problems setting volume"); WARNING("problems setting alsa volume: %s\n", snd_strerror(err)); closeAlsaMixer(); @@ -469,7 +462,7 @@ int getVolumeLevel(void) } } -static int changeSoftwareVolume(mpd_unused int fd, int change, int rel) +static int changeSoftwareVolume(int change, int rel) { int new = change; @@ -497,19 +490,19 @@ static int changeSoftwareVolume(mpd_unused int fd, int change, int rel) return 0; } -int changeVolumeLevel(int fd, int change, int rel) +int changeVolumeLevel(int change, int rel) { switch (volume_mixerType) { #ifdef HAVE_ALSA case VOLUME_MIXER_TYPE_ALSA: - return changeAlsaVolumeLevel(fd, change, rel); + return changeAlsaVolumeLevel(change, rel); #endif #ifdef HAVE_OSS case VOLUME_MIXER_TYPE_OSS: - return changeOssVolumeLevel(fd, change, rel); + return changeOssVolumeLevel(change, rel); #endif case VOLUME_MIXER_TYPE_SOFTWARE: - return changeSoftwareVolume(fd, change, rel); + return changeSoftwareVolume(change, rel); default: return 0; } @@ -531,7 +524,7 @@ void read_sw_volume_state(FILE *fp) continue; sv = strtol(buf + len, &end, 10); if (mpd_likely(!*end)) - changeSoftwareVolume(STDERR_FILENO, sv, 0); + changeSoftwareVolume(sv, 0); else ERROR("Can't parse software volume: %s\n", buf); return; diff --git a/src/volume.h b/src/volume.h index 85a9eefa8..a92cdd8bb 100644 --- a/src/volume.h +++ b/src/volume.h @@ -33,7 +33,7 @@ void finishVolume(void); int getVolumeLevel(void); -int changeVolumeLevel(int fd, int change, int rel); +int changeVolumeLevel(int change, int rel); void read_sw_volume_state(FILE *fp); -- cgit v1.2.3 From 2a6a8a35c6b31e3c83891076dab86dc2da2d82ae Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:51:50 +0200 Subject: audio: don't pass "fd" to {en,dis}ableAudioDevice() No protocol code in the audio output library. --- src/audio.c | 17 +++++------------ src/audio.h | 4 ++-- src/command.c | 18 ++++++++++++++---- 3 files changed, 21 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/audio.c b/src/audio.c index 9a65df858..d4447bc7e 100644 --- a/src/audio.c +++ b/src/audio.c @@ -19,9 +19,7 @@ #include "audio.h" #include "audioOutput.h" #include "log.h" -#include "command.h" #include "path.h" -#include "ack.h" #include "myfprintf.h" #include "os_compat.h" @@ -428,13 +426,10 @@ void sendMetadataToAudioDevice(const struct mpd_tag *tag) } } -int enableAudioDevice(int fd, unsigned int device) +int enableAudioDevice(unsigned int device) { - if (device >= audioOutputArraySize) { - commandError(fd, ACK_ERROR_ARG, "audio output device id %i " - "doesn't exist\n", device); + if (device >= audioOutputArraySize) return -1; - } if (!(audioDeviceStates[device] & 0x01)) audioDeviceStates[device] = DEVICE_ENABLE; @@ -442,13 +437,11 @@ int enableAudioDevice(int fd, unsigned int device) return 0; } -int disableAudioDevice(int fd, unsigned int device) +int disableAudioDevice(unsigned int device) { - if (device >= audioOutputArraySize) { - commandError(fd, ACK_ERROR_ARG, "audio output device id %i " - "doesn't exist\n", device); + if (device >= audioOutputArraySize) return -1; - } + if (audioDeviceStates[device] & 0x01) audioDeviceStates[device] = DEVICE_DISABLE; diff --git a/src/audio.h b/src/audio.h index 92cb9cf11..7ff0d5d4a 100644 --- a/src/audio.h +++ b/src/audio.h @@ -59,9 +59,9 @@ void sendMetadataToAudioDevice(const struct mpd_tag *tag); /* these functions are called in the main parent process while the child process is busy playing to the audio */ -int enableAudioDevice(int fd, unsigned int device); +int enableAudioDevice(unsigned int device); -int disableAudioDevice(int fd, unsigned int device); +int disableAudioDevice(unsigned int device); void printAudioDevices(int fd); diff --git a/src/command.c b/src/command.c index dc62481ff..b67a57522 100644 --- a/src/command.c +++ b/src/command.c @@ -1137,21 +1137,31 @@ static int handleCrossfade(int fd, mpd_unused int *permission, static int handleEnableDevice(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - int device; + int device, ret; if (check_int(fd, &device, argv[1], check_non_negative, argv[1]) < 0) return -1; - return enableAudioDevice(fd, device); + + ret = enableAudioDevice(device); + if (ret == -1) + commandError(fd, ACK_ERROR_NO_EXIST, "No such audio output"); + + return ret; } static int handleDisableDevice(int fd, mpd_unused int *permission, mpd_unused int argc, char *argv[]) { - int device; + int device, ret; if (check_int(fd, &device, argv[1], check_non_negative, argv[1]) < 0) return -1; - return disableAudioDevice(fd, device); + + ret = disableAudioDevice(device); + if (ret == -1) + commandError(fd, ACK_ERROR_NO_EXIST, "No such audio output"); + + return ret; } static int handleDevices(int fd, mpd_unused int *permission, -- cgit v1.2.3 From f6a9a891177815af39a7100ab90ccb942552b85f Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 13:57:43 +0200 Subject: command: concatenate strings at compile time String literals (including those defined in CPP macros) can be concatenated at compile time. This saves some CPU cycles in vsnprintf() at run time. --- src/command.c | 60 ++++++++++++++++++++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 27 deletions(-) (limited to 'src') diff --git a/src/command.c b/src/command.c index b67a57522..1da3736d1 100644 --- a/src/command.c +++ b/src/command.c @@ -376,44 +376,50 @@ static int commandStatus(mpd_unused int fd, mpd_unused int *permission, break; } - fdprintf(fd, "%s: %i\n", COMMAND_STATUS_VOLUME, getVolumeLevel()); - fdprintf(fd, "%s: %i\n", COMMAND_STATUS_REPEAT, - getPlaylistRepeatStatus()); - fdprintf(fd, "%s: %i\n", COMMAND_STATUS_RANDOM, - getPlaylistRandomStatus()); - fdprintf(fd, "%s: %li\n", COMMAND_STATUS_PLAYLIST, - getPlaylistVersion()); - fdprintf(fd, "%s: %i\n", COMMAND_STATUS_PLAYLIST_LENGTH, - getPlaylistLength()); - fdprintf(fd, "%s: %i\n", COMMAND_STATUS_CROSSFADE, - (int)(ob_get_xfade() + 0.5)); - - fdprintf(fd, "%s: %s\n", COMMAND_STATUS_STATE, state); + fdprintf(fd, + COMMAND_STATUS_VOLUME ": %i\n" + COMMAND_STATUS_REPEAT ": %i\n" + COMMAND_STATUS_RANDOM ": %i\n" + COMMAND_STATUS_PLAYLIST ": %li\n" + COMMAND_STATUS_PLAYLIST_LENGTH ": %i\n" + COMMAND_STATUS_CROSSFADE ": %i\n" + COMMAND_STATUS_STATE ": %s\n", + getVolumeLevel(), + getPlaylistRepeatStatus(), + getPlaylistRandomStatus(), + getPlaylistVersion(), + getPlaylistLength(), + (int)(ob_get_xfade() + 0.5), + state); song = getPlaylistCurrentSong(); if (song >= 0) { - fdprintf(fd, "%s: %i\n", COMMAND_STATUS_SONG, song); - fdprintf(fd, "%s: %i\n", COMMAND_STATUS_SONGID, - getPlaylistSongId(song)); + fdprintf(fd, + COMMAND_STATUS_SONG ": %i\n" + COMMAND_STATUS_SONGID ": %i\n", + song, getPlaylistSongId(song)); } if (ob_get_state() != OB_STATE_STOP) { - fdprintf(fd, "%s: %lu:%lu\n", COMMAND_STATUS_TIME, - ob_get_elapsed_time(), ob_get_total_time()); - fdprintf(fd, "%s: %u\n", COMMAND_STATUS_BITRATE, - ob_get_bit_rate()); - fdprintf(fd, "%s: %u:%u:%u\n", COMMAND_STATUS_AUDIO, - ob_get_sample_rate(), ob_get_bits(), - ob_get_channels()); + fdprintf(fd, + COMMAND_STATUS_TIME ": %lu:%lu\n" + COMMAND_STATUS_BITRATE ": %u\n" + COMMAND_STATUS_AUDIO ": %u:%u:%u\n", + ob_get_elapsed_time(), + ob_get_total_time(), + ob_get_bit_rate(), + ob_get_sample_rate(), + ob_get_bits(), + ob_get_channels()); } if ((updateJobId = isUpdatingDB())) { - fdprintf(fd, "%s: %i\n", COMMAND_STATUS_UPDATING_DB, - updateJobId); + fdprintf(fd, COMMAND_STATUS_UPDATING_DB ": %i\n", + updateJobId); } if (player_errno != PLAYER_ERROR_NONE) { - fdprintf(fd, "%s: %s\n", COMMAND_STATUS_ERROR, - player_strerror()); + fdprintf(fd, COMMAND_STATUS_ERROR ": %s\n", + player_strerror()); } return 0; -- cgit v1.2.3 From 56c11abbf5a5a6e7b53905f91fda1ba75b02b296 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 19:19:48 +0200 Subject: playlist: return -1 after assert(0) print_playlist_result() had an assert(0) at the end, in case there was an invalid result value. With NDEBUG, this resulted in a function not returning a value - add a dummy "return -1" at the end to keep gcc quiet. --- src/command.c | 1 + 1 file changed, 1 insertion(+) (limited to 'src') diff --git a/src/command.c b/src/command.c index 1da3736d1..ebca41bde 100644 --- a/src/command.c +++ b/src/command.c @@ -263,6 +263,7 @@ static int print_playlist_result(int fd, enum playlist_result result) } assert(0); + return -1; } static void addCommand(const char *name, -- cgit v1.2.3 From d5187ce694cca1c5bd05bd562d9494e7387a86d0 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 7 Sep 2008 19:14:39 +0200 Subject: fix -Wcast-qual -Wwrite-strings warnings The previous patch enabled these warnings. In Eric's branch, they were worked around with a generic deconst_ptr() function. There are several places where we can add "const" to pointers, and in others, libraries want non-const strings. In the latter, convert string literals to "static char[]" variables - this takes the same space, and seems safer than deconsting a string literal. --- src/audioOutputs/audioOutput_jack.c | 17 +++++++++-------- src/audioOutputs/audioOutput_shout.c | 4 ++-- src/inputPlugins/mod_plugin.c | 17 +++++++++++++---- src/inputPlugins/wavpack_plugin.c | 12 ++++++++---- src/pcm_utils.c | 19 ++++++++++--------- 5 files changed, 42 insertions(+), 27 deletions(-) (limited to 'src') diff --git a/src/audioOutputs/audioOutput_jack.c b/src/audioOutputs/audioOutput_jack.c index 8818bb739..77ee077a4 100644 --- a/src/audioOutputs/audioOutput_jack.c +++ b/src/audioOutputs/audioOutput_jack.c @@ -31,8 +31,8 @@ static const size_t sample_size = sizeof(jack_default_audio_sample_t); typedef struct _JackData { /* configuration */ - char *name; - char *output_ports[2]; + const char *name; + const char *output_ports[2]; int ringbuf_sz; /* locks */ @@ -52,6 +52,7 @@ typedef struct _JackData { static JackData *newJackData(void) { JackData *ret; + ret = xcalloc(sizeof(JackData), 1); ret->name = "mpd"; @@ -97,12 +98,12 @@ static void freeJackData(AudioOutput *audioOutput) freeJackClient(jd); if (strcmp(jd->name, "mpd") != 0) - free(jd->name); + free(deconst_ptr(jd->name)); for ( i = ARRAY_SIZE(jd->output_ports); --i >= 0; ) { if (!jd->output_ports[i]) continue; - free(jd->output_ports[i]); + free(deconst_ptr(jd->output_ports[i])); } free(jd); @@ -267,7 +268,7 @@ static int jack_testDefault(void) static int connect_jack(AudioOutput *audioOutput) { JackData *jd = audioOutput->data; - char **jports; + const char **jports; char *port_name; if ( (jd->client = jack_client_new(jd->name)) == NULL ) { @@ -304,9 +305,9 @@ static int connect_jack(AudioOutput *audioOutput) /* hay que buscar que hay */ if ( !jd->output_ports[1] - && (jports = (char **)jack_get_ports(jd->client, NULL, NULL, - JackPortIsPhysical| - JackPortIsInput)) ) { + && (jports = jack_get_ports(jd->client, NULL, NULL, + JackPortIsPhysical| + JackPortIsInput)) ) { jd->output_ports[0] = jports[0]; jd->output_ports[1] = jports[1] ? jports[1] : jports[0]; DEBUG("output_ports: %s %s\n", diff --git a/src/audioOutputs/audioOutput_shout.c b/src/audioOutputs/audioOutput_shout.c index b65a8253f..2ee942809 100644 --- a/src/audioOutputs/audioOutput_shout.c +++ b/src/audioOutputs/audioOutput_shout.c @@ -636,8 +636,8 @@ static int myShout_play(AudioOutput * audioOutput, for (i = 0; i < samples; i++) { for (j = 0; j < sd->audioFormat->channels; j++) { - vorbbuf[j][i] = - (*((mpd_sint16 *)deconst_ptr(playChunk))) / 32768.0; + vorbbuf[j][i] = (*((const mpd_sint16 *) playChunk)) + / 32768.0; playChunk += bytes; } } diff --git a/src/inputPlugins/mod_plugin.c b/src/inputPlugins/mod_plugin.c index 586d87ae9..6b09e3cf0 100644 --- a/src/inputPlugins/mod_plugin.c +++ b/src/inputPlugins/mod_plugin.c @@ -48,14 +48,21 @@ static BOOL mod_mpd_IsThere(void) return 1; } +static char drv_name[] = "MPD"; +static char drv_version[] = "MPD Output Driver v0.1"; + +#if (LIBMIKMOD_VERSION > 0x030106) +static char drv_alias[] = "mpd"; +#endif + static MDRIVER drv_mpd = { NULL, - "MPD", - "MPD Output Driver v0.1", + drv_name, + drv_version, 0, 255, #if (LIBMIKMOD_VERSION > 0x030106) - "mpd", /* Alias */ + drv_alias, #if (LIBMIKMOD_VERSION >= 0x030200) NULL, /* CmdLineHelp */ #endif @@ -92,6 +99,8 @@ static int mod_mikModInitError; static int mod_initMikMod(void) { + static char params[] = ""; + if (mod_mikModInitError) return -1; @@ -110,7 +119,7 @@ static int mod_initMikMod(void) md_mode = (DMODE_SOFT_MUSIC | DMODE_INTERP | DMODE_STEREO | DMODE_16BITS); - if (MikMod_Init("")) { + if (MikMod_Init(params)) { ERROR("Could not init MikMod: %s\n", MikMod_strerror(MikMod_errno)); mod_mikModInitError = 1; diff --git a/src/inputPlugins/wavpack_plugin.c b/src/inputPlugins/wavpack_plugin.c index 77339dabd..8bba8ae2b 100644 --- a/src/inputPlugins/wavpack_plugin.c +++ b/src/inputPlugins/wavpack_plugin.c @@ -224,34 +224,38 @@ static char *wavpack_tag(WavpackContext *wpc, char *key) static ReplayGainInfo *wavpack_replaygain(WavpackContext *wpc) { + static char replaygain_track_gain[] = "replaygain_track_gain"; + static char replaygain_album_gain[] = "replaygain_album_gain"; + static char replaygain_track_peak[] = "replaygain_track_peak"; + static char replaygain_album_peak[] = "replaygain_album_peak"; ReplayGainInfo *replayGainInfo; int found = 0; char *value; replayGainInfo = newReplayGainInfo(); - value = wavpack_tag(wpc, "replaygain_track_gain"); + value = wavpack_tag(wpc, replaygain_track_gain); if (value) { replayGainInfo->trackGain = atof(value); free(value); found = 1; } - value = wavpack_tag(wpc, "replaygain_album_gain"); + value = wavpack_tag(wpc, replaygain_album_gain); if (value) { replayGainInfo->albumGain = atof(value); free(value); found = 1; } - value = wavpack_tag(wpc, "replaygain_track_peak"); + value = wavpack_tag(wpc, replaygain_track_peak); if (value) { replayGainInfo->trackPeak = atof(value); free(value); found = 1; } - value = wavpack_tag(wpc, "replaygain_album_peak"); + value = wavpack_tag(wpc, replaygain_album_peak); if (value) { replayGainInfo->albumPeak = atof(value); free(value); diff --git a/src/pcm_utils.c b/src/pcm_utils.c index f716c279d..90856fa1d 100644 --- a/src/pcm_utils.c +++ b/src/pcm_utils.c @@ -149,6 +149,7 @@ static int pcm_getSampleRateConverter(void) const char *conf = getConfigParamValue(CONF_SAMPLERATE_CONVERTER); long convalgo; char *test; + const char *test2; size_t len; if (!conf) { @@ -162,12 +163,12 @@ static int pcm_getSampleRateConverter(void) len = strlen(conf); for (convalgo = 0 ; ; convalgo++) { - test = (char *)src_get_name(convalgo); - if (!test) { + test2 = src_get_name(convalgo); + if (!test2) { convalgo = SRC_SINC_FASTEST; break; } - if (strncasecmp(test, conf, len) == 0) + if (strncasecmp(test2, conf, len) == 0) goto out; } @@ -239,7 +240,7 @@ static size_t pcm_convertSampleRate(mpd_sint8 channels, mpd_uint32 inSampleRate, data->data_out = xrealloc(data->data_out, dataOutSize); } - src_short_to_float_array((short *)inBuffer, data->data_in, + src_short_to_float_array((const short *)inBuffer, data->data_in, data->input_frames * channels); error = src_process(convState->state, data); @@ -305,7 +306,7 @@ static char *pcm_convertChannels(mpd_sint8 channels, const char *inBuffer, static char *buf; static size_t len; char *outBuffer = NULL; - mpd_sint16 *in; + const mpd_sint16 *in; mpd_sint16 *out; int inSamples, i; @@ -320,7 +321,7 @@ static char *pcm_convertChannels(mpd_sint8 channels, const char *inBuffer, outBuffer = buf; inSamples = inSize >> 1; - in = (mpd_sint16 *)inBuffer; + in = (const mpd_sint16 *)inBuffer; out = (mpd_sint16 *)outBuffer; for (i = 0; i < inSamples; i++) { *out++ = *in; @@ -338,7 +339,7 @@ static char *pcm_convertChannels(mpd_sint8 channels, const char *inBuffer, outBuffer = buf; inSamples = inSize >> 2; - in = (mpd_sint16 *)inBuffer; + in = (const mpd_sint16 *)inBuffer; out = (mpd_sint16 *)outBuffer; for (i = 0; i < inSamples; i++) { *out = (*in++) / 2; @@ -359,7 +360,7 @@ static const char *pcm_convertTo16bit(mpd_sint8 bits, const char *inBuffer, static char *buf; static size_t len; char *outBuffer = NULL; - mpd_sint8 *in; + const mpd_sint8 *in; mpd_sint16 *out; size_t i; @@ -372,7 +373,7 @@ static const char *pcm_convertTo16bit(mpd_sint8 bits, const char *inBuffer, } outBuffer = buf; - in = (mpd_sint8 *)inBuffer; + in = (const mpd_sint8 *)inBuffer; out = (mpd_sint16 *)outBuffer; for (i = 0; i < inSize; i++) *out++ = (*in++) << 8; -- cgit v1.2.3 From e82e9cd8b2bebbaf096bf3a091d3ea0e9613797b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 6 Sep 2008 15:31:55 +0200 Subject: client: removed assert(client->fd)>=0 Since client->fd==-1 has become our "expired" flag, it may already be -1 when client_close() is called. Don't assert that it is still non-negative, and call client_set_expired() instead. --- src/client.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 6a43b9586..6bda52bd5 100644 --- a/src/client.c +++ b/src/client.c @@ -234,15 +234,13 @@ static void client_close(struct client *client) { struct sllnode *buf; - assert(client->fd >= 0); - - xclose(client->fd); - assert(num_clients > 0); assert(!list_empty(&clients)); list_del(&client->siblings); --num_clients; + client_set_expired(client); + if (client->cmd_list) { free_cmd_list(client->cmd_list); client->cmd_list = NULL; -- cgit v1.2.3 From 2c0e478e371d14df47d7a7f31515bbd1091f5b6a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sat, 6 Sep 2008 15:31:55 +0200 Subject: client: don't check FD_ISSET(client->fd) on expired client client->fd becomes -1 when the client expires. Don't use FD_ISSET() with this expired client; doing so would cause a crash due to SIGBUS. --- src/client.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 6bda52bd5..d04cac31f 100644 --- a/src/client.c +++ b/src/client.c @@ -536,7 +536,8 @@ int client_manager_io(void) } client->lastTime = time(NULL); } - if (FD_ISSET(client->fd, &wfds)) { + if (!client_is_expired(client) && + FD_ISSET(client->fd, &wfds)) { client_write_deferred(client); client->lastTime = time(NULL); } -- cgit v1.2.3 From 44fc077f3bc6855bc83acaae9d43cf1e7ccc749a Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Sep 2008 11:41:33 +0200 Subject: client: moved CLOSE/KILL check after client_process_line() Don't update client data if it is going to be closed anyway. --- src/client.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index d04cac31f..615e67ace 100644 --- a/src/client.c +++ b/src/client.c @@ -419,6 +419,9 @@ static int client_input_received(struct client *client, int bytesRead) *(buf_tail - 1) = '\0'; } ret = client_process_line(client); + if (ret == COMMAND_RETURN_KILL || + ret == COMMAND_RETURN_CLOSE) + return ret; if (client_is_expired(client)) return ret; client->bufferPos = client->bufferLength; @@ -442,10 +445,6 @@ static int client_input_received(struct client *client, int bytesRead) client->bufferLength); client->bufferPos = 0; } - if (ret == COMMAND_RETURN_KILL || ret == COMMAND_RETURN_CLOSE) { - return ret; - } - } return ret; -- cgit v1.2.3 From 6452dab44c6a170d8081b1a15223fb28ea9ab230 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Sep 2008 11:41:34 +0200 Subject: client: renamed local variable "selret" to "ret" It's easier to reuse the variable if it has a more generic name. --- src/client.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 615e67ace..9b70116bd 100644 --- a/src/client.c +++ b/src/client.c @@ -506,7 +506,7 @@ int client_manager_io(void) fd_set wfds; fd_set efds; struct client *client, *n; - int selret; + int ret; int fdmax = 0; FD_ZERO( &efds ); @@ -515,15 +515,16 @@ int client_manager_io(void) registered_IO_add_fds(&fdmax, &rfds, &wfds, &efds); - selret = select(fdmax + 1, &rfds, &wfds, &efds, NULL); - if (selret < 0) { + ret = select(fdmax + 1, &rfds, &wfds, &efds, NULL); + + if (ret < 0) { if (errno == EINTR) return 0; FATAL("select() failed: %s\n", strerror(errno)); } - registered_IO_consume_fds(&selret, &rfds, &wfds, &efds); + registered_IO_consume_fds(&ret, &rfds, &wfds, &efds); getConnections(&rfds); -- cgit v1.2.3 From 70199776b4c925268105df7998905ee36c79846b Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Sep 2008 11:42:26 +0200 Subject: client: check for COMMAND_RETURN_CLOSE Don't close the client within client_process_line(), return COMMAND_RETURN_CLOSE instead. This is the signal for the caller chain to actually close it. This makes dealing with the client pointer a lot safer, since the caller always knows whether it is still valid. --- src/client.c | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 9b70116bd..03e9ad0dc 100644 --- a/src/client.c +++ b/src/client.c @@ -342,10 +342,8 @@ static int client_process_line(struct client *client) "list returned %i\n", client->num, ret); if (ret == COMMAND_RETURN_CLOSE || - client_is_expired(client)) { - client_close(client); + client_is_expired(client)) return COMMAND_RETURN_CLOSE; - } if (ret == 0) commandSuccess(client->fd); @@ -367,8 +365,7 @@ static int client_process_line(struct client *client) (unsigned long)client->cmd_list_size, (unsigned long) client_max_command_list_size); - client_close(client); - ret = COMMAND_RETURN_CLOSE; + return COMMAND_RETURN_CLOSE; } else new_cmd_list_ptr(client, line, len); } @@ -388,10 +385,8 @@ static int client_process_line(struct client *client) client->num, ret); if (ret == COMMAND_RETURN_CLOSE || - client_is_expired(client)) { - client_close(client); + client_is_expired(client)) return COMMAND_RETURN_CLOSE; - } if (ret == 0) commandSuccess(client->fd); @@ -422,16 +417,14 @@ static int client_input_received(struct client *client, int bytesRead) if (ret == COMMAND_RETURN_KILL || ret == COMMAND_RETURN_CLOSE) return ret; - if (client_is_expired(client)) - return ret; + assert(!client_is_expired(client)); client->bufferPos = client->bufferLength; } if (client->bufferLength == CLIENT_MAX_BUFFER_LENGTH) { if (client->bufferPos == 0) { ERROR("client %i: buffer overflow\n", client->num); - client_close(client); - return 1; + return COMMAND_RETURN_CLOSE; } if (client->cmd_list_OK >= 0 && client->cmd_list && @@ -461,7 +454,7 @@ static int client_read(struct client *client) if (bytesRead > 0) return client_input_received(client, bytesRead); else if (bytesRead == 0 || (bytesRead < 0 && errno != EINTR)) { - client_close(client); + return COMMAND_RETURN_CLOSE; } else return 0; @@ -530,10 +523,16 @@ int client_manager_io(void) list_for_each_entry_safe(client, n, &clients, siblings) { if (FD_ISSET(client->fd, &rfds)) { - if (COMMAND_RETURN_KILL == - client_read(client)) { + ret = client_read(client); + if (ret == COMMAND_RETURN_KILL) return COMMAND_RETURN_KILL; + if (ret == COMMAND_RETURN_CLOSE) { + client_close(client); + continue; } + + assert(!client_is_expired(client)); + client->lastTime = time(NULL); } if (!client_is_expired(client) && -- cgit v1.2.3 From 929e0e6d351a4a231d3b7aeb08df571be5d3b949 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Sep 2008 11:42:30 +0200 Subject: client: client_input_received() returns 0 Since the caller chain doesn't care about the return value (except for COMMAND_RETURN_KILL, COMMAND_RETURN_CLOSE), just return 0 if there is nothing special. This saves one local variable initialization, and one access to it. Also remove one unreachable "return 1" from client_read(). --- src/client.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 03e9ad0dc..381b4713b 100644 --- a/src/client.c +++ b/src/client.c @@ -400,7 +400,7 @@ static int client_process_line(struct client *client) static int client_input_received(struct client *client, int bytesRead) { - int ret = 0; + int ret; char *buf_tail = &(client->buffer[client->bufferLength - 1]); while (bytesRead > 0) { @@ -440,7 +440,7 @@ static int client_input_received(struct client *client, int bytesRead) } } - return ret; + return 0; } static int client_read(struct client *client) @@ -457,8 +457,6 @@ static int client_read(struct client *client) return COMMAND_RETURN_CLOSE; } else return 0; - - return 1; } static void client_manager_register_read_fd(fd_set * fds, int *fdmax) -- cgit v1.2.3 From e63e56c4e3a0d902dada9d47298517afc1aa16a3 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Wed, 10 Sep 2008 11:43:09 +0200 Subject: client: simplified client_read() Remove one comparison by changing branch order. --- src/client.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index 381b4713b..ce8d731a6 100644 --- a/src/client.c +++ b/src/client.c @@ -453,10 +453,12 @@ static int client_read(struct client *client) if (bytesRead > 0) return client_input_received(client, bytesRead); - else if (bytesRead == 0 || (bytesRead < 0 && errno != EINTR)) { - return COMMAND_RETURN_CLOSE; - } else + else if (bytesRead < 0 && errno == EINTR) + /* try again later, after select() */ return 0; + else + /* peer disconnected or I/O error */ + return COMMAND_RETURN_CLOSE; } static void client_manager_register_read_fd(fd_set * fds, int *fdmax) -- cgit v1.2.3 From cb0253d02dc1d51bc84c65d8c7362c8a18c116f2 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Fri, 12 Sep 2008 01:25:18 -0700 Subject: client: shorten names of the struct client variables Seeing the token "client" repeatedly in the same blocks of code adds to mental fatigue and makes it harder to follow code because there's fewer unique tokens to distinguish. "cl" is unique within mpd and conveys enough information to be useful to anybody reading the code. --- src/client.c | 419 +++++++++++++++++++++++++++++------------------------------ 1 file changed, 209 insertions(+), 210 deletions(-) (limited to 'src') diff --git a/src/client.c b/src/client.c index ce8d731a6..c3ed7fec5 100644 --- a/src/client.c +++ b/src/client.c @@ -87,17 +87,17 @@ struct client { static LIST_HEAD(clients); static unsigned num_clients; -static void client_write_deferred(struct client *client); +static void client_write_deferred(struct client *cl); -static void client_write_output(struct client *client); +static void client_write_output(struct client *cl); #ifdef SO_SNDBUF -static size_t get_default_snd_buf_size(struct client *client) +static size_t get_default_snd_buf_size(struct client *cl) { int new_size; socklen_t sockOptLen = sizeof(int); - if (getsockopt(client->fd, SOL_SOCKET, SO_SNDBUF, + if (getsockopt(cl->fd, SOL_SOCKET, SO_SNDBUF, (char *)&new_size, &sockOptLen) < 0) { DEBUG("problem getting sockets send buffer size\n"); return CLIENT_DEFAULT_OUT_BUFFER_SIZE; @@ -108,67 +108,67 @@ static size_t get_default_snd_buf_size(struct client *client) return CLIENT_DEFAULT_OUT_BUFFER_SIZE; } #else /* !SO_SNDBUF */ -static size_t get_default_snd_buf_size(struct client *client) +static size_t get_default_snd_buf_size(struct client *cl) { return CLIENT_DEFAULT_OUT_BUFFER_SIZE; } #endif /* !SO_SNDBUF */ -static void set_send_buf_size(struct client *client) +static void set_send_buf_size(struct client *cl) { - size_t new_size = get_default_snd_buf_size(client); - if (client->send_buf_size != new_size) { - client->send_buf_size = new_size; + size_t new_size = get_default_snd_buf_size(cl); + if (cl->send_buf_size != new_size) { + cl->send_buf_size = new_size; /* don't resize to get smaller, only bigger */ - if (client->send_buf_alloc < new_size) { - if (client->send_buf) - free(client->send_buf); - client->send_buf = xmalloc(new_size); - client->send_buf_alloc = new_size; + if (cl->send_buf_alloc < new_size) { + if (cl->send_buf) + free(cl->send_buf); + cl->send_buf = xmalloc(new_size); + cl->send_buf_alloc = new_size; } } } -static inline int client_is_expired(const struct client *client) +static inline int client_is_expired(const struct client *cl) { - return client->fd < 0; + return cl->fd < 0; } static int global_expired; -static inline void client_set_expired(struct client *client) +static inline void client_set_expired(struct client *cl) { - if (client->fd >= 0) { - xclose(client->fd); - client->fd = -1; + if (cl->fd >= 0) { + xclose(cl->fd); + cl->fd = -1; } global_expired = 1; } -static void client_init(struct client *client, int fd) +static void client_init(struct client *cl, int fd) { static unsigned int next_client_num; assert(fd >= 0); - client->cmd_list_size = 0; - client->cmd_list_dup = 0; - client->cmd_list_OK = -1; - client->bufferLength = 0; - client->bufferPos = 0; - client->fd = fd; + cl->cmd_list_size = 0; + cl->cmd_list_dup = 0; + cl->cmd_list_OK = -1; + cl->bufferLength = 0; + cl->bufferPos = 0; + cl->fd = fd; set_nonblocking(fd); - client->lastTime = time(NULL); - client->cmd_list = NULL; - client->cmd_list_tail = NULL; - client->deferred_send = NULL; - client->deferred_bytes = 0; - client->num = next_client_num++; - client->send_buf_used = 0; + cl->lastTime = time(NULL); + cl->cmd_list = NULL; + cl->cmd_list_tail = NULL; + cl->deferred_send = NULL; + cl->deferred_bytes = 0; + cl->num = next_client_num++; + cl->send_buf_used = 0; - client->permission = getDefaultPermissions(); - set_send_buf_size(client); + cl->permission = getDefaultPermissions(); + set_send_buf_size(cl); xwrite(fd, GREETING, strlen(GREETING)); } @@ -189,25 +189,25 @@ static void free_cmd_list(struct strnode *list) } } -static void cmd_list_clone(struct client *client) +static void cmd_list_clone(struct client *cl) { - struct strnode *new = dup_strlist(client->cmd_list); - free_cmd_list(client->cmd_list); - client->cmd_list = new; - client->cmd_list_dup = 1; + struct strnode *new = dup_strlist(cl->cmd_list); + free_cmd_list(cl->cmd_list); + cl->cmd_list = new; + cl->cmd_list_dup = 1; /* new tail */ while (new && new->next) new = new->next; - client->cmd_list_tail = new; + cl->cmd_list_tail = new; } -static void new_cmd_list_ptr(struct client *client, char *s, const int size) +static void new_cmd_list_ptr(struct client *cl, char *s, const int size) { int i; struct strnode *new; - if (!client->cmd_list_dup) { + if (!cl->cmd_list_dup) { for (i = client_list_cache_size - 1; i >= 0; --i) { if (list_cache[i].data) continue; @@ -220,46 +220,46 @@ static void new_cmd_list_ptr(struct client *client, char *s, const int size) } /* allocate from the heap */ - new = client->cmd_list_dup ? new_strnode_dup(s, size) + new = cl->cmd_list_dup ? new_strnode_dup(s, size) : new_strnode(s); out: - if (client->cmd_list) { - client->cmd_list_tail->next = new; - client->cmd_list_tail = new; + if (cl->cmd_list) { + cl->cmd_list_tail->next = new; + cl->cmd_list_tail = new; } else - client->cmd_list = client->cmd_list_tail = new; + cl->cmd_list = cl->cmd_list_tail = new; } -static void client_close(struct client *client) +static void client_close(struct client *cl) { struct sllnode *buf; assert(num_clients > 0); assert(!list_empty(&clients)); - list_del(&client->siblings); + list_del(&cl->siblings); --num_clients; - client_set_expired(client); + client_set_expired(cl); - if (client->cmd_list) { - free_cmd_list(client->cmd_list); - client->cmd_list = NULL; + if (cl->cmd_list) { + free_cmd_list(cl->cmd_list); + cl->cmd_list = NULL; } - if ((buf = client->deferred_send)) { + if ((buf = cl->deferred_send)) { do { struct sllnode *prev = buf; buf = buf->next; free(prev); } while (buf); - client->deferred_send = NULL; + cl->deferred_send = NULL; } - if (client->send_buf) - free(client->send_buf); + if (cl->send_buf) + free(cl->send_buf); - SECURE("client %i: closed\n", client->num); - free(client); + SECURE("client %i: closed\n", cl->num); + free(cl); } static const char * @@ -306,7 +306,7 @@ sockaddr_to_tmp_string(const struct sockaddr *addr) void client_new(int fd, const struct sockaddr *addr) { - struct client *client; + struct client *cl; if (num_clients >= client_max_connections) { ERROR("Max Connections Reached!\n"); @@ -314,145 +314,145 @@ void client_new(int fd, const struct sockaddr *addr) return; } - client = xcalloc(1, sizeof(*client)); - list_add(&client->siblings, &clients); + cl = xcalloc(1, sizeof(struct client)); + list_add(&cl->siblings, &clients); ++num_clients; - client_init(client, fd); - SECURE("client %i: opened from %s\n", client->num, + client_init(cl, fd); + SECURE("client %i: opened from %s\n", cl->num, sockaddr_to_tmp_string(addr)); } -static int client_process_line(struct client *client) +static int client_process_line(struct client *cl) { int ret = 1; - char *line = client->buffer + client->bufferPos; + char *line = cl->buffer + cl->bufferPos; - if (client->cmd_list_OK >= 0) { + if (cl->cmd_list_OK >= 0) { if (strcmp(line, CLIENT_LIST_MODE_END) == 0) { DEBUG("client %i: process command " - "list\n", client->num); + "list\n", cl->num); global_expired = 0; - ret = processListOfCommands(client->fd, - &(client->permission), + ret = processListOfCommands(cl->fd, + &(cl->permission), &global_expired, - client->cmd_list_OK, - client->cmd_list); + cl->cmd_list_OK, + cl->cmd_list); DEBUG("client %i: process command " - "list returned %i\n", client->num, ret); + "list returned %i\n", cl->num, ret); if (ret == COMMAND_RETURN_CLOSE || - client_is_expired(client)) + client_is_expired(cl)) return COMMAND_RETURN_CLOSE; if (ret == 0) - commandSuccess(client->fd); + commandSuccess(cl->fd); - client_write_output(client); - free_cmd_list(client->cmd_list); - client->cmd_list = NULL; - client->cmd_list_OK = -1; + client_write_output(cl); + free_cmd_list(cl->cmd_list); + cl->cmd_list = NULL; + cl->cmd_list_OK = -1; } else { size_t len = strlen(line) + 1; - client->cmd_list_size += len; - if (client->cmd_list_size > + cl->cmd_list_size += len; + if (cl->cmd_list_size > client_max_command_list_size) { ERROR("client %i: command " "list size (%lu) is " "larger than the max " "(%lu)\n", - client->num, - (unsigned long)client->cmd_list_size, + cl->num, + (unsigned long)cl->cmd_list_size, (unsigned long) client_max_command_list_size); return COMMAND_RETURN_CLOSE; } else - new_cmd_list_ptr(client, line, len); + new_cmd_list_ptr(cl, line, len); } } else { if (strcmp(line, CLIENT_LIST_MODE_BEGIN) == 0) { - client->cmd_list_OK = 0; + cl->cmd_list_OK = 0; ret = 1; } else if (strcmp(line, CLIENT_LIST_OK_MODE_BEGIN) == 0) { - client->cmd_list_OK = 1; + cl->cmd_list_OK = 1; ret = 1; } else { DEBUG("client %i: process command \"%s\"\n", - client->num, line); - ret = processCommand(client->fd, - &(client->permission), line); + cl->num, line); + ret = processCommand(cl->fd, + &(cl->permission), line); DEBUG("client %i: command returned %i\n", - client->num, ret); + cl->num, ret); if (ret == COMMAND_RETURN_CLOSE || - client_is_expired(client)) + client_is_expired(cl)) return COMMAND_RETURN_CLOSE; if (ret == 0) - commandSuccess(client->fd); + commandSuccess(cl->fd); - client_write_output(client); + client_write_output(cl); } } return ret; } -static int client_input_received(struct client *client, int bytesRead) +static int client_input_received(struct client *cl, int bytesRead) { int ret; - char *buf_tail = &(client->buffer[client->bufferLength - 1]); + char *buf_tail = &(cl->buffer[cl->bufferLength - 1]); while (bytesRead > 0) { - client->bufferLength++; + cl->bufferLength++; bytesRead--; buf_tail++; if (*buf_tail == '\n') { *buf_tail = '\0'; - if (client->bufferLength > client->bufferPos) { + if (cl->bufferLength > cl->bufferPos) { if (*(buf_tail - 1) == '\r') *(buf_tail - 1) = '\0'; } - ret = client_process_line(client); + ret = client_process_line(cl); if (ret == COMMAND_RETURN_KILL || ret == COMMAND_RETURN_CLOSE) return ret; - assert(!client_is_expired(client)); - client->bufferPos = client->bufferLength; + assert(!client_is_expired(cl)); + cl->bufferPos = cl->bufferLength; } - if (client->bufferLength == CLIENT_MAX_BUFFER_LENGTH) { - if (client->bufferPos == 0) { + if (cl->bufferLength == CLIENT_MAX_BUFFER_LENGTH) { + if (cl->bufferPos == 0) { ERROR("client %i: buffer overflow\n", - client->num); + cl->num); return COMMAND_RETURN_CLOSE; } - if (client->cmd_list_OK >= 0 && - client->cmd_list && - !client->cmd_list_dup) - cmd_list_clone(client); - assert(client->bufferLength >= client->bufferPos + if (cl->cmd_list_OK >= 0 && + cl->cmd_list && + !cl->cmd_list_dup) + cmd_list_clone(cl); + assert(cl->bufferLength >= cl->bufferPos && "bufferLength >= bufferPos"); - client->bufferLength -= client->bufferPos; - memmove(client->buffer, - client->buffer + client->bufferPos, - client->bufferLength); - client->bufferPos = 0; + cl->bufferLength -= cl->bufferPos; + memmove(cl->buffer, + cl->buffer + cl->bufferPos, + cl->bufferLength); + cl->bufferPos = 0; } } return 0; } -static int client_read(struct client *client) +static int client_read(struct client *cl) { int bytesRead; - bytesRead = read(client->fd, - client->buffer + client->bufferLength, - CLIENT_MAX_BUFFER_LENGTH - client->bufferLength); + bytesRead = read(cl->fd, + cl->buffer + cl->bufferLength, + CLIENT_MAX_BUFFER_LENGTH - cl->bufferLength); if (bytesRead > 0) - return client_input_received(client, bytesRead); + return client_input_received(cl, bytesRead); else if (bytesRead < 0 && errno == EINTR) /* try again later, after select() */ return 0; @@ -463,32 +463,32 @@ static int client_read(struct client *client) static void client_manager_register_read_fd(fd_set * fds, int *fdmax) { - struct client *client; + struct client *cl; FD_ZERO(fds); addListenSocketsToFdSet(fds, fdmax); - list_for_each_entry(client, &clients, siblings) { - if (!client_is_expired(client) && !client->deferred_send) { - FD_SET(client->fd, fds); - if (*fdmax < client->fd) - *fdmax = client->fd; + list_for_each_entry(cl, &clients, siblings) { + if (!client_is_expired(cl) && !cl->deferred_send) { + FD_SET(cl->fd, fds); + if (*fdmax < cl->fd) + *fdmax = cl->fd; } } } static void client_manager_register_write_fd(fd_set * fds, int *fdmax) { - struct client *client; + struct client *cl; FD_ZERO(fds); - list_for_each_entry(client, &clients, siblings) { - if (client->fd >= 0 && !client_is_expired(client) - && client->deferred_send) { - FD_SET(client->fd, fds); - if (*fdmax < client->fd) - *fdmax = client->fd; + list_for_each_entry(cl, &clients, siblings) { + if (cl->fd >= 0 && !client_is_expired(cl) + && cl->deferred_send) { + FD_SET(cl->fd, fds); + if (*fdmax < cl->fd) + *fdmax = cl->fd; } } } @@ -498,7 +498,7 @@ int client_manager_io(void) fd_set rfds; fd_set wfds; fd_set efds; - struct client *client, *n; + struct client *cl, *n; int ret; int fdmax = 0; @@ -521,24 +521,24 @@ int client_manager_io(void) getConnections(&rfds); - list_for_each_entry_safe(client, n, &clients, siblings) { - if (FD_ISSET(client->fd, &rfds)) { - ret = client_read(client); + list_for_each_entry_safe(cl, n, &clients, siblings) { + if (FD_ISSET(cl->fd, &rfds)) { + ret = client_read(cl); if (ret == COMMAND_RETURN_KILL) return COMMAND_RETURN_KILL; if (ret == COMMAND_RETURN_CLOSE) { - client_close(client); + client_close(cl); continue; } - assert(!client_is_expired(client)); + assert(!client_is_expired(cl)); - client->lastTime = time(NULL); + cl->lastTime = time(NULL); } - if (!client_is_expired(client) && - FD_ISSET(client->fd, &wfds)) { - client_write_deferred(client); - client->lastTime = time(NULL); + if (!client_is_expired(cl) && + FD_ISSET(cl->fd, &wfds)) { + client_write_deferred(cl); + cl->lastTime = time(NULL); } } @@ -601,10 +601,10 @@ void client_manager_init(void) static void client_close_all(void) { - struct client *client, *n; + struct client *cl, *n; - list_for_each_entry_safe(client, n, &clients, siblings) - client_close(client); + list_for_each_entry_safe(cl, n, &clients, siblings) + client_close(cl); num_clients = 0; free(list_cache); @@ -619,69 +619,69 @@ void client_manager_deinit(void) void client_manager_expire(void) { - struct client *client, *n; + struct client *cl, *n; - list_for_each_entry_safe(client, n, &clients, siblings) { - if (client_is_expired(client)) { - DEBUG("client %i: expired\n", client->num); - client_close(client); - } else if (time(NULL) - client->lastTime > + list_for_each_entry_safe(cl, n, &clients, siblings) { + if (client_is_expired(cl)) { + DEBUG("client %i: expired\n", cl->num); + client_close(cl); + } else if (time(NULL) - cl->lastTime > client_timeout) { - DEBUG("client %i: timeout\n", client->num); - client_close(client); + DEBUG("client %i: timeout\n", cl->num); + client_close(cl); } } } -static void client_write_deferred(struct client *client) +static void client_write_deferred(struct client *cl) { struct sllnode *buf; ssize_t ret = 0; - buf = client->deferred_send; + buf = cl->deferred_send; while (buf) { assert(buf->size > 0); - ret = write(client->fd, buf->data, buf->size); + ret = write(cl->fd, buf->data, buf->size); if (ret < 0) break; else if ((size_t)ret < buf->size) { - assert(client->deferred_bytes >= (size_t)ret); - client->deferred_bytes -= ret; + assert(cl->deferred_bytes >= (size_t)ret); + cl->deferred_bytes -= ret; buf->data = (char *)buf->data + ret; buf->size -= ret; } else { struct sllnode *tmp = buf; size_t decr = (buf->size + sizeof(struct sllnode)); - assert(client->deferred_bytes >= decr); - client->deferred_bytes -= decr; + assert(cl->deferred_bytes >= decr); + cl->deferred_bytes -= decr; buf = buf->next; free(tmp); - client->deferred_send = buf; + cl->deferred_send = buf; } - client->lastTime = time(NULL); + cl->lastTime = time(NULL); } - if (!client->deferred_send) { - DEBUG("client %i: buffer empty %lu\n", client->num, - (unsigned long)client->deferred_bytes); - assert(client->deferred_bytes == 0); + if (!cl->deferred_send) { + DEBUG("client %i: buffer empty %lu\n", cl->num, + (unsigned long)cl->deferred_bytes); + assert(cl->deferred_bytes == 0); } else if (ret < 0 && errno != EAGAIN && errno != EINTR) { /* cause client to close */ DEBUG("client %i: problems flushing buffer\n", - client->num); - client_set_expired(client); + cl->num); + client_set_expired(cl); } } -static struct client *client_by_fd(int fd) +static struct client * client_by_fd(int fd) { - struct client *client; + struct client *cl; - list_for_each_entry(client, &clients, siblings) - if (client->fd == fd) - return client; + list_for_each_entry(cl, &clients, siblings) + if (cl->fd == fd) + return cl; return NULL; } @@ -689,97 +689,96 @@ static struct client *client_by_fd(int fd) int client_print(int fd, const char *buffer, size_t buflen) { size_t copylen; - struct client *client; + struct client *cl; assert(fd >= 0); - client = client_by_fd(fd); - if (client == NULL) + cl = client_by_fd(fd); + if (cl == NULL) return -1; /* if fd isn't found or client is going to be closed, do nothing */ - if (client_is_expired(client)) + if (client_is_expired(cl)) return 0; - while (buflen > 0 && !client_is_expired(client)) { + while (buflen > 0 && !client_is_expired(cl)) { size_t left; - assert(client->send_buf_size >= client->send_buf_used); - left = client->send_buf_size - client->send_buf_used; + assert(cl->send_buf_size >= cl->send_buf_used); + left = cl->send_buf_size - cl->send_buf_used; copylen = buflen > left ? left : buflen; - memcpy(client->send_buf + client->send_buf_used, buffer, + memcpy(cl->send_buf + cl->send_buf_used, buffer, copylen); buflen -= copylen; - client->send_buf_used += copylen; + cl->send_buf_used += copylen; buffer += copylen; - if (client->send_buf_used >= client->send_buf_size) - client_write_output(client); + if (cl->send_buf_used >= cl->send_buf_size) + client_write_output(cl); } return 0; } -static void client_defer_output(struct client *client, +static void client_defer_output(struct client *cl, const void *data, size_t length) { struct sllnode **buf_r; assert(length > 0); - client->deferred_bytes += sizeof(struct sllnode) + length; - if (client->deferred_bytes > client_max_output_buffer_size) { + cl->deferred_bytes += sizeof(struct sllnode) + length; + if (cl->deferred_bytes > client_max_output_buffer_size) { ERROR("client %i: output buffer size (%lu) is " "larger than the max (%lu)\n", - client->num, - (unsigned long)client->deferred_bytes, + cl->num, + (unsigned long)cl->deferred_bytes, (unsigned long)client_max_output_buffer_size); /* cause client to close */ - client_set_expired(client); + client_set_expired(cl); return; } - buf_r = &client->deferred_send; + buf_r = &cl->deferred_send; while (*buf_r != NULL) buf_r = &(*buf_r)->next; *buf_r = new_sllnode(data, length); } -static void client_write(struct client *client, +static void client_write(struct client *cl, const char *data, size_t length) { ssize_t ret; assert(length > 0); - assert(client->deferred_send == NULL); + assert(cl->deferred_send == NULL); - if ((ret = write(client->fd, data, length)) < 0) { + if ((ret = write(cl->fd, data, length)) < 0) { if (errno == EAGAIN || errno == EINTR) { - client_defer_output(client, data, length); + client_defer_output(cl, data, length); } else { - DEBUG("client %i: problems writing\n", client->num); - client_set_expired(client); + DEBUG("client %i: problems writing\n", cl->num); + client_set_expired(cl); return; } - } else if ((size_t)ret < client->send_buf_used) { - client_defer_output(client, data + ret, length - ret); + } else if ((size_t)ret < cl->send_buf_used) { + client_defer_output(cl, data + ret, length - ret); } - if (client->deferred_send) - DEBUG("client %i: buffer created\n", client->num); + if (cl->deferred_send) + DEBUG("client %i: buffer created\n", cl->num); } -static void client_write_output(struct client *client) +static void client_write_output(struct client *cl) { - if (client_is_expired(client) || !client->send_buf_used) + if (client_is_expired(cl) || !cl->send_buf_used) return; - if (client->deferred_send != NULL) - client_defer_output(client, client->send_buf, - client->send_buf_used); + if (cl->deferred_send != NULL) + client_defer_output(cl, cl->send_buf, cl->send_buf_used); else - client_write(client, client->send_buf, client->send_buf_used); + client_write(cl, cl->send_buf, cl->send_buf_used); - client->send_buf_used = 0; + cl->send_buf_used = 0; } -- cgit v1.2.3