From: Andre Noll Date: Sun, 26 Feb 2012 13:23:59 +0000 (+0100) Subject: Merge branch 't/mmap_sanity' X-Git-Tag: v0.4.10~10 X-Git-Url: http://git.tuebingen.mpg.de/?p=paraslash.git;a=commitdiff_plain;h=26a0b7e457d1dd7e0e40d866d28de00c5d1cfa54;hp=-c Merge branch 't/mmap_sanity' Conflicts: error.h --- 26a0b7e457d1dd7e0e40d866d28de00c5d1cfa54 diff --combined NEWS index f14f2879,16e8e4e8..e5ae590e --- a/NEWS +++ b/NEWS @@@ -2,13 -2,6 +2,15 @@@ 0.4.10 (to be announced) "heterogeneous vacuum" ----------------------------------------------- + - The --no_default_filters option of para_filter has been + depricated. It still works but has no effect and will be + removed in the next version. - - - Cleanup and consolidation of the various wrappers for - write(), writev(), send() and friends. ++ - Cleanup and consolidation of the various wrappers for ++ write(), writev(), send() and friends. ++ - The obscure error messages on mmap() failures have been ++ replaced by meaningful messages. This affects mainly ++ para_afh. + ------------------------------------- 0.4.9 (2011-12-06) "hybrid causality" ------------------------------------- diff --combined afh.c index b76c72fd,04c13ebd..bffe6321 --- a/afh.c +++ b/afh.c @@@ -124,7 -124,7 +124,7 @@@ static int cat_file(struct afh_info *af &header, &size); if (size > 0) { PARA_INFO_LOG("writing header (%zu bytes)\n", size); - ret = write(STDOUT_FILENO, header, size); /* FIXME */ + ret = write_all(STDOUT_FILENO, header, size); afh_free_header(header, audio_format_id); if (ret < 0) return ret; @@@ -152,7 -152,7 +152,7 @@@ if (!size) continue; PARA_INFO_LOG("writing chunk %lu\n", i); - ret = write_all(STDOUT_FILENO, buf, &size); + ret = write_all(STDOUT_FILENO, buf, size); if (ret < 0) return ret; } @@@ -187,8 -187,10 +187,10 @@@ int main(int argc, char **argv int ret2; ret = mmap_full_file(conf.inputs[i], O_RDONLY, &audio_file_data, &audio_file_size, &fd); - if (ret < 0) + if (ret < 0) { + PARA_ERROR_LOG("failed to mmap \"%s\"\n", conf.inputs[i]); goto out; + } ret = compute_afhi(conf.inputs[i], audio_file_data, audio_file_size, fd, &afhi); if (ret < 0) diff --combined error.h index f7216ea3,39c109f9..e34d8094 --- a/error.h +++ b/error.h @@@ -441,7 -441,7 +441,8 @@@ extern const char **para_errlist[] PARA_ERROR(FGETS, "fgets error"), \ PARA_ERROR(EOF, "end of file"), \ PARA_ERROR(READ_PATTERN, "did not read expected pattern"), \ + PARA_ERROR(SHORT_WRITE, "unexpected short write"), \ + PARA_ERROR(EMPTY, "file is empty"), \ #define ALSA_WRITE_ERRORS \ diff --combined fd.c index a04232f8,9b6c6917..3a8406d9 --- a/fd.c +++ b/fd.c @@@ -19,136 -19,60 +19,136 @@@ #include "fd.h" /** - * Write a buffer to a file descriptor, re-write on short writes. + * Write an array of buffers to a file descriptor. * * \param fd The file descriptor. - * \param buf The buffer to be sent. - * \param len The length of \a buf. + * \param iov Pointer to one or more buffers. + * \param iovcnt The number of buffers. + * + * EAGAIN/EWOULDBLOCK is not considered a fatal error condition. For example + * DCCP CCID3 has a sending wait queue which fills up and is emptied + * asynchronously. The EAGAIN case means that there is currently no space in + * the wait queue, but this can change at any moment. + * + * \return Negative on fatal errors, number of bytes written else. * - * \return Standard. In any case, the number of bytes that have been written is - * stored in \a len. + * For blocking file descriptors, this function returns either the sum of all + * buffer sizes, or the error code of the fatal error that caused the last + * write call to fail. + * + * For nonblocking file descriptors there is a third possibility: Any positive + * return value less than the sum of the buffer sizes indicates that some bytes + * have been written but the next write would block. + * + * \sa writev(2), \ref xwrite(). */ -int write_all(int fd, const char *buf, size_t *len) +int xwritev(int fd, struct iovec *iov, int iovcnt) { - size_t total = *len; + size_t written = 0; + int i; + struct iovec saved_iov, *curiov; - assert(total); - *len = 0; - while (*len < total) { - int ret = write(fd, buf + *len, total - *len); - if (ret == -1) - return -ERRNO_TO_PARA_ERROR(errno); - *len += ret; + i = 0; + curiov = iov; + saved_iov = *curiov; + while (i < iovcnt && curiov->iov_len > 0) { + ssize_t ret = writev(fd, curiov, iovcnt - i); + if (ret >= 0) { + written += ret; + while (ret > 0) { + if (ret < curiov->iov_len) { + curiov->iov_base += ret; + curiov->iov_len -= ret; + break; + } + ret -= curiov->iov_len; + *curiov = saved_iov; + i++; + if (i >= iovcnt) + return written; + curiov++; + saved_iov = *curiov; + } + continue; + } + if (errno == EINTR) + /* + * The write() call was interrupted by a signal before + * any data was written. Try again. + */ + continue; + if (errno == EAGAIN || errno == EWOULDBLOCK) + /* + * We don't consider this an error. Note that POSIX + * allows either error to be returned, and does not + * require these constants to have the same value. + */ + return written; + /* fatal error */ + return -ERRNO_TO_PARA_ERROR(errno); } - return 1; + return written; } /** - * Write a buffer to a non-blocking file descriptor. + * Write a buffer to a file descriptor, re-writing on short writes. * * \param fd The file descriptor. - * \param buf the buffer to write. - * \param len the number of bytes of \a buf. + * \param buf The buffer to write. + * \param len The number of bytes to write. * - * EAGAIN is not considered an error condition. For example CCID3 has a - * sending wait queue which fills up and is emptied asynchronously. The EAGAIN - * case means that there is currently no space in the wait queue, but this can - * change at any moment. + * This is a simple wrapper for \ref xwritev(). * - * \return Negative on errors, number of bytes written else. + * \return The return value of the underlying call to \ref xwritev(). */ -int write_nonblock(int fd, const char *buf, size_t len) +int xwrite(int fd, const char *buf, size_t len) { - size_t written = 0; - int ret = 0; + struct iovec iov = {.iov_base = (void *)buf, .iov_len = len}; + return xwritev(fd, &iov, 1); +} + +/** + * Write all data to a file descriptor. + * + * \param fd The file descriptor. + * \param buf The buffer to be sent. + * \param len The length of \a buf. + * + * This is like \ref xwrite() but returns \p -E_SHORT_WRITE if not + * all data could be written. + * + * \return Number of bytes written on success, negative error code else. + */ +int write_all(int fd, const char *buf, size_t len) +{ + int ret = xwrite(fd, buf, len); - while (written < len) { - size_t num = len - written; + if (ret < 0) + return ret; + if (ret != len) + return -E_SHORT_WRITE; + return ret; +} - ret = write(fd, buf + written, num); - if (ret < 0 && errno == EAGAIN) - return written; - if (ret < 0) - return -ERRNO_TO_PARA_ERROR(errno); - written += ret; - } - return written; +/** + * Write a buffer given by a format string. + * + * \param fd The file descriptor. + * \param fmt A format string. + * + * \return The return value of the underlying call to \ref write_all(). + */ +__printf_2_3 int write_va_buffer(int fd, const char *fmt, ...) +{ + char *msg; + int ret; + va_list ap; + + va_start(ap, fmt); + ret = xvasprintf(&msg, fmt, ap); + ret = write_all(fd, msg, ret); + free(msg); + return ret; } /** @@@ -163,7 -87,7 +163,7 @@@ * If \a rfds is not \p NULL and the (non-blocking) file descriptor \a fd is * not set in \a rfds, this function returns early without doing anything. * Otherwise The function tries to read up to \a sz bytes from \a fd. As for - * write_nonblock(), EAGAIN is not considered an error condition. However, EOF + * xwrite(), EAGAIN is not considered an error condition. However, EOF * is. * * \return Zero or a negative error code. If the underlying call to readv(2) @@@ -176,7 -100,7 +176,7 @@@ * have been read before the error occurred. In this case \a num_bytes is * positive. * - * \sa \ref write_nonblock(), read(2), readv(2). + * \sa \ref xwrite(), read(2), readv(2). */ int readv_nonblock(int fd, struct iovec *iov, int iovcnt, fd_set *rfds, size_t *num_bytes) @@@ -617,6 -541,22 +617,22 @@@ int mmap_full_file(const char *path, in goto out; } *size = file_status.st_size; + /* + * If the file is empty, *size is zero and mmap() would return EINVAL + * (Invalid argument). This error is common enough to spend an extra + * error code which explicitly states the problem. + */ + ret = -E_EMPTY; + if (*size == 0) + goto out; + /* + * If fd refers to a directory, mmap() returns ENODEV (No such device), + * at least on Linux. "Is a directory" seems to be more to the point. + */ + ret = -ERRNO_TO_PARA_ERROR(EISDIR); + if (S_ISDIR(file_status.st_mode)) + goto out; + ret = para_mmap(*size, mmap_prot, mmap_flags, fd, 0, map); out: if (ret < 0 || !fd_ptr)