From f77f0e0341220e10d1732404346bd2c1fe2c6835 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Sun, 25 Nov 2007 12:08:24 +0100 Subject: [PATCH] Get rid of E_SEND. This patch also simplifies sendall() a bit and fixes two bugs in add_one_audio_file(): - If the check for existence of a hash sister failed, we missed to unmap the audio file. - If an error occurred we send out a message to the client. If this message can not be sent (because the client terminated the connection) we returned success anyway and happily tried to add the next audio file. Some documentation improvements as well. --- aft.c | 36 +++++++++++++++++------------------ error.h | 1 - net.c | 58 +++++++++++++++++++++++++-------------------------------- 3 files changed, 42 insertions(+), 53 deletions(-) diff --git a/aft.c b/aft.c index 40c7e106..c54e6afb 100644 --- a/aft.c +++ b/aft.c @@ -1652,7 +1652,7 @@ static int hash_sister_callback(const struct osl_object *query, static int add_one_audio_file(const char *path, const void *private_data) { - int ret, ret2; + int ret, send_ret = 1; uint8_t format_num = -1; const struct private_add_data *pad = private_data; struct afh_info afhi, *afhi_ptr = NULL; @@ -1677,7 +1677,7 @@ static int add_one_audio_file(const char *path, const void *private_data) ret = 1; if (pb && (pad->flags & ADD_FLAG_LAZY)) { /* lazy is really cheap */ if (pad->flags & ADD_FLAG_VERBOSE) - ret = send_va_buffer(pad->fd, "lazy-ignore: %s\n", path); + send_ret = send_va_buffer(pad->fd, "lazy-ignore: %s\n", path); goto out_free; } /* We still want to add this file. Compute its hash. */ @@ -1686,27 +1686,27 @@ static int add_one_audio_file(const char *path, const void *private_data) goto out_free; hash_function(map.data, map.size, hash); - /* Check whether database contains file with the same hash. */ + /* Check whether the database contains a file with the same hash. */ query.data = hash; query.size = HASH_SIZE; ret = send_callback_request(hash_sister_callback, &query, &result); if (ret < 0 && ret != -E_RB_KEY_NOT_FOUND) - goto out_free; + goto out_unmap; if (ret >= 0) { hs = *(struct osl_row **)result.data; free(result.data); } /* Return success if we already know this file. */ ret = 1; - if (pb && hs && hs == pb && (!(pad->flags & ADD_FLAG_FORCE))) { + if (pb && hs && hs == pb && !(pad->flags & ADD_FLAG_FORCE)) { if (pad->flags & ADD_FLAG_VERBOSE) - ret = send_va_buffer(pad->fd, + send_ret = send_va_buffer(pad->fd, "%s exists, not forcing update\n", path); goto out_unmap; } /* - * we won't recalculate the audio format info and the chunk table if - * there is a hash sister unless in FORCE mode. + * We won't recalculate the audio format info and the chunk table if + * there is a hash sister and FORCE was not given. */ if (!hs || (pad->flags & ADD_FLAG_FORCE)) { ret = compute_afhi(path, map.data, map.size, &afhi); @@ -1715,34 +1715,32 @@ static int add_one_audio_file(const char *path, const void *private_data) format_num = ret; afhi_ptr = &afhi; } + munmap(map.data, map.size); if (pad->flags & ADD_FLAG_VERBOSE) { - ret = send_va_buffer(pad->fd, "adding %s\n", path); - if (ret < 0) - goto out_unmap; + send_ret = send_va_buffer(pad->fd, "adding %s\n", path); + if (send_ret < 0) + goto out_free; } - munmap(map.data, map.size); save_audio_file_info(hash, path, afhi_ptr, pad->flags, format_num, &obj); /* Ask afs to consider this entry for adding. */ ret = send_callback_request(com_add_callback, &obj, &result); if (ret > 0) { - ret2 = send_va_buffer(pad->fd, "%s", (char *)result.data); + send_ret = send_va_buffer(pad->fd, "%s", (char *)result.data); free(result.data); - if (ret >= 0 && ret2 < 0) - ret = ret2; } goto out_free; out_unmap: munmap(map.data, map.size); out_free: - if (ret < 0 && ret != -E_SEND) - send_va_buffer(pad->fd, "failed to add %s (%s)\n", path, + if (ret < 0 && send_ret >= 0) + send_ret = send_va_buffer(pad->fd, "failed to add %s (%s)\n", path, PARA_STRERROR(-ret)); free(obj.data); if (afhi_ptr) free(afhi_ptr->chunk_table); - /* it's not an error if not all files could be added */ - return ret == -E_SEND? ret : 1; + /* Stop adding files only on send errors. */ + return send_ret; } int com_add(int fd, int argc, char * const * const argv) diff --git a/error.h b/error.h index c420f490..07821c22 100644 --- a/error.h +++ b/error.h @@ -173,7 +173,6 @@ extern const char **para_errlist[]; #define NET_ERRORS \ - PARA_ERROR(SEND, "send error"), \ PARA_ERROR(SOCKET, "socket error"), \ PARA_ERROR(CONNECT, "connect error"), \ PARA_ERROR(ACCEPT, "accept error"), \ diff --git a/net.c b/net.c index 2991b6db..f01cf583 100644 --- a/net.c +++ b/net.c @@ -95,36 +95,28 @@ void init_sockaddr(struct sockaddr_in *addr, int port, const struct hostent *he) } /* - * send out a buffer, resend on short writes + * Send out a buffer, resend on short writes. * - * \param fd the file descriptor - * \param buf The buffer to be sent - * \param len The length of \a buf - * - * Due to circumstances beyond your control, the kernel might not send all the - * data out in one chunk, and now, my friend, it's up to us to get the data out - * there (Beej's Guide to Network Programming). + * \param fd The file descriptor. + * \param buf The buffer to be sent. + * \param len The length of \a buf. * - * \return This function returns 1 on success and \a -E_SEND on errors. The - * number of bytes actually sent is stored upon successful return in \a len. + * \return Standard. In any case, the number of bytes actually sent is stored + * in \a len. */ static int sendall(int fd, const char *buf, size_t *len) { - size_t total = 0, bytesleft = *len; /* how many we have left to send */ - int n = -1; - - while (total < *len) { - n = send(fd, buf + total, bytesleft, 0); - if (n == -1) - break; - total += n; - bytesleft -= n; - if (total < *len) - PARA_DEBUG_LOG("short write (%zd byte(s) left)\n", - *len - total); + size_t total = *len; + + assert(total); + *len = 0; + while (*len < total) { + int ret = send(fd, buf + *len, total - *len, 0); + if (ret == -1) + return -ERRNO_TO_PARA_ERROR(errno); + *len += ret; } - *len = total; /* return number actually sent here */ - return n == -1? -E_SEND : 1; /* return 1 on success */ + return 1; } /** @@ -138,7 +130,7 @@ static int sendall(int fd, const char *buf, size_t *len) * out the buffer, encrypted or not, and try to resend the remaing part in case * of short writes. * - * \return Positive on success, \p -E_SEND on errors. + * \return Standard. */ int send_bin_buffer(int fd, const char *buf, size_t len) { @@ -162,14 +154,14 @@ int send_bin_buffer(int fd, const char *buf, size_t len) } /** - * encrypt and send null terminated buffer. + * Encrypt and send null terminated buffer. * - * \param fd the file descriptor - * \param buf the null-terminated buffer to be send + * \param fd The file descriptor. + * \param buf The null-terminated buffer to be send. * * This is equivalent to send_bin_buffer(fd, buf, strlen(buf)). * - * \return Positive on success, \p -E_SEND on errors. + * \return Standard. */ int send_buffer(int fd, const char *buf) { @@ -178,12 +170,12 @@ int send_buffer(int fd, const char *buf) /** - * send and encrypt a buffer given by a format string + * Send and encrypt a buffer given by a format string. * - * \param fd the file descriptor - * \param fmt a format string + * \param fd The file descriptor. + * \param fmt A format string. * - * \return Positive on success, \p -E_SEND on errors. + * \return Standard. */ __printf_2_3 int send_va_buffer(int fd, const char *fmt, ...) { -- 2.39.5