From: Andre Noll Date: Sun, 6 Aug 2023 16:38:05 +0000 (+0200) Subject: Merge topic branch t/fd into master X-Git-Tag: v0.7.3~13 X-Git-Url: http://git.tuebingen.mpg.de/?a=commitdiff_plain;h=420a1f30cc06482f36371d096635846f8800e198;hp=7e1d89b8086f022ab2a7e5ceedba2acd8c964b52;p=paraslash.git Merge topic branch t/fd into master A rash of patches which clean up a good part of fd.c. Nothing major here, mostly simplifications and documentation improvements. * refs/heads/t/fd: fd: Simplify and move for_each_file_in_dir(). fd.c: Improve error checking of para_mkdir(). fd: Revamp para_mkdir(). fd: Improve read_pattern(), rename it to read_and_compare(). fd: Remove log message from para_munmap(). fd: Open-code para_chdir(). fd: Remove file_exists(). fd: Improve documentation of xwritev(). fd: Improve documentation of write_all(). fd: Improve documentation of write_va_buffer(). --- diff --git a/NEWS.md b/NEWS.md index a259ef3e..a0484f09 100644 --- a/NEWS.md +++ b/NEWS.md @@ -11,6 +11,7 @@ NEWS - Version 1.0 of the openssl library has been deprecated. A warning is printed at compile-time on systems which have this outdated version because it will no longer be supported once paraslash-0.8.0 comes out. +- A spring cleanup for the senescent code in fd.c. Downloads: [tarball](./releases/paraslash-git.tar.xz) diff --git a/afs.c b/afs.c index 83889bb9..865effde 100644 --- a/afs.c +++ b/afs.c @@ -580,17 +580,6 @@ static void get_database_dir(void) PARA_INFO_LOG("afs_database dir %s\n", database_dir); } -static int make_database_dir(void) -{ - int ret; - - get_database_dir(); - ret = para_mkdir(database_dir, 0777); - if (ret >= 0 || ret == -ERRNO_TO_PARA_ERROR(EEXIST)) - return 1; - return ret; -} - static int open_afs_tables(void) { int i, ret; @@ -1062,7 +1051,8 @@ static int com_init(struct command_context *cc, struct lls_parse_result *lpr) .size = sizeof(table_mask)}; unsigned num_inputs = lls_num_inputs(lpr); - ret = make_database_dir(); + get_database_dir(); + ret = para_mkdir(database_dir); if (ret < 0) return ret; if (num_inputs > 0) { diff --git a/aft.c b/aft.c index b4d8c2dd..0aa9c6ba 100644 --- a/aft.c +++ b/aft.c @@ -6,6 +6,7 @@ #include #include #include +#include #include #include @@ -1906,6 +1907,53 @@ out_free: return send_ret; } +/* + * Call back once for each regular file below a directory. + * + * Traverse the given directory recursively and call the supplied callback for + * each regular file encountered. The first argument to the callback will be + * the path to the regular file and the second argument will be the data + * pointer. All file types except regular files and directories are ignored. In + * particular, symlinks are not followed. Subdirectories are ignored silently + * if the calling process has insufficient access permissions. + */ +static int for_each_file_in_dir(const char *dirname, + int (*func)(const char *, void *), void *data) +{ + int ret; + DIR *dir; + struct dirent *entry; + + dir = opendir(dirname); + if (!dir) + return errno == EACCES? 1 : -ERRNO_TO_PARA_ERROR(errno); + /* scan cwd recursively */ + while ((entry = readdir(dir))) { + char *tmp; + struct stat s; + + if (!strcmp(entry->d_name, ".")) + continue; + if (!strcmp(entry->d_name, "..")) + continue; + tmp = make_message("%s/%s", dirname, entry->d_name); + ret = 0; + if (lstat(tmp, &s) != -1) { + if (S_ISREG(s.st_mode)) + ret = func(tmp, data); + else if (S_ISDIR(s.st_mode)) + ret = for_each_file_in_dir(tmp, func, data); + } + free(tmp); + if (ret < 0) + goto out; + } + ret = 1; +out: + closedir(dir); + return ret; +} + static int com_add(struct command_context *cc, struct lls_parse_result *lpr) { int i, ret; diff --git a/client_common.c b/client_common.c index 43527ddb..3b90000f 100644 --- a/client_common.c +++ b/client_common.c @@ -578,8 +578,9 @@ int client_parse_config(int argc, char *argv[], struct client_task **ct_ptr, if (CLIENT_OPT_GIVEN(KEY_FILE, lpr)) kf = para_strdup(CLIENT_OPT_STRING_VAL(KEY_FILE, lpr)); else { + struct stat statbuf; kf = make_message("%s/.paraslash/key.%s", home, user); - if (!file_exists(kf)) { + if (stat(kf, &statbuf) != 0) { /* assume file does not exist */ free(kf); kf = make_message("%s/.ssh/id_rsa", home); } diff --git a/fd.c b/fd.c index 763f756c..1af902f9 100644 --- a/fd.c +++ b/fd.c @@ -37,26 +37,27 @@ int xrename(const char *oldpath, const char *newpath) } /** - * Write an array of buffers to a file descriptor. + * Write an array of buffers, handling non-fatal errors. * - * \param fd The file descriptor. + * \param fd The file descriptor to write to. * \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. + * EAGAIN, EWOULDBLOCK and EINTR are not considered error conditions. If a + * write operation fails with EAGAIN or EWOULDBLOCK, the number of bytes that + * have been written so far is returned. In the EINTR case the operation is + * retried. Short writes are handled by issuing a subsequent write operation + * for the remaining part. * * \return Negative on fatal errors, number of bytes written else. * * 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. + * buffer sizes or a negative error code which indicates the fatal error that + * caused a 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. + * For nonblocking file descriptors there is a third possibility: Any + * non-negative return value less than the sum of the buffer sizes indicates + * that a write operation returned EAGAIN/EWOULDBLOCK. * * \sa writev(2), \ref xwrite(). */ @@ -126,14 +127,15 @@ int xwrite(int fd, const char *buf, size_t len) } /** - * Write all data to a file descriptor. + * Write to a file descriptor, fail on short writes. * * \param fd The file descriptor. - * \param buf The buffer to be sent. - * \param len The length of \a buf. + * \param buf The buffer to be written. + * \param len The length of the buffer. * - * This is like \ref xwrite() but returns \p -E_SHORT_WRITE if not - * all data could be written. + * For blocking file descriptors this function behaves identical to \ref + * xwrite(). For non-blocking file descriptors it returns -E_SHORT_WRITE + * (rather than a value less than len) if not all data could be written. * * \return Number of bytes written on success, negative error code else. */ @@ -149,12 +151,20 @@ int write_all(int fd, const char *buf, size_t len) } /** - * Write a buffer given by a format string. + * A fprintf-like function for raw file descriptors. + * + * This function creates a string buffer according to the given format and + * writes this buffer to a file descriptor. * * \param fd The file descriptor. * \param fmt A format string. * + * The difference to fprintf(3) is that the first argument is a file + * descriptor, not a FILE pointer. This function does not rely on stdio. + * * \return The return value of the underlying call to \ref write_all(). + * + * \sa fprintf(3), \ref xvasprintf(). */ __printf_2_3 int write_va_buffer(int fd, const char *fmt, ...) { @@ -250,64 +260,44 @@ int read_nonblock(int fd, void *buf, size_t sz, size_t *num_bytes) } /** - * Read a buffer and check its content for a pattern. - * - * \param fd The file descriptor to receive from. - * \param pattern The expected pattern. - * \param bufsize The size of the internal buffer. + * Read a buffer and compare its contents to a string, ignoring case. * - * This function tries to read at most \a bufsize bytes from the non-blocking - * file descriptor \a fd. If at least \p strlen(\a pattern) bytes have been - * received, the beginning of the received buffer is compared with \a pattern, - * ignoring case. + * \param fd The file descriptor to read from. + * \param expectation The expected string to compare to. * - * \return Positive if \a pattern was received, negative on errors, zero if no data - * was available to read. + * The given file descriptor is expected to be in non-blocking mode. The string + * comparison is performed using strncasecmp(3). * - * \sa \ref read_nonblock(), \sa strncasecmp(3). + * \return Zero if no data was available, positive if a buffer was read whose + * contents compare as equal to the expected string, negative otherwise. + * Possible errors: (a) not enough data was read, (b) the buffer contents + * compared as non-equal, (c) a read error occurred. In the first two cases, + * -E_READ_PATTERN is returned. In the read error case the (negative) return + * value of the underlying call to \ref read_nonblock() is returned. */ -int read_pattern(int fd, const char *pattern, size_t bufsize) +int read_and_compare(int fd, const char *expectation) { - size_t n, len; - char *buf = alloc(bufsize + 1); - int ret = read_nonblock(fd, buf, bufsize, &n); + size_t n, len = strlen(expectation); + char *buf = alloc(len + 1); + int ret = read_nonblock(fd, buf, len, &n); - buf[n] = '\0'; if (ret < 0) goto out; + buf[n] = '\0'; ret = 0; if (n == 0) goto out; ret = -E_READ_PATTERN; - len = strlen(pattern); if (n < len) goto out; - if (strncasecmp(buf, pattern, len) != 0) + if (strncasecmp(buf, expectation, len) != 0) goto out; ret = 1; out: - if (ret < 0) { - PARA_NOTICE_LOG("%s\n", para_strerror(-ret)); - PARA_NOTICE_LOG("recvd %zu bytes: %s\n", n, buf); - } free(buf); return ret; } -/** - * Check whether a file exists. - * - * \param fn The file name. - * - * \return True iff file exists. - */ -bool file_exists(const char *fn) -{ - struct stat statbuf; - - return !stat(fn, &statbuf); -} - /** * Set a file descriptor to blocking mode. * @@ -397,86 +387,32 @@ int para_open(const char *path, int flags, mode_t mode) } /** - * Wrapper for chdir(2). - * - * \param path The specified directory. + * Create a directory, don't fail if it already exists. * - * \return Standard. - */ -int para_chdir(const char *path) -{ - int ret = chdir(path); - - if (ret >= 0) - return 1; - return -ERRNO_TO_PARA_ERROR(errno); -} - -/** - * Save the cwd and open a given directory. - * - * \param dirname Path to the directory to open. - * \param dir Result pointer. - * \param cwd File descriptor of the current working directory. - * - * \return Standard. - * - * Opening the current directory (".") and calling fchdir() to return is - * usually faster and more reliable than saving cwd in some buffer and calling - * chdir() afterwards. - * - * If \a cwd is not \p NULL "." is opened and the resulting file descriptor is - * stored in \a cwd. If the function returns success, and \a cwd is not \p - * NULL, the caller must close this file descriptor (probably after calling - * fchdir(*cwd)). - * - * On errors, the function undos everything, so the caller needs neither close - * any files, nor change back to the original working directory. + * \param path Name of the directory to create. * - * \sa getcwd(3). + * This function passes the fixed mode value 0777 to mkdir(3) (which consults + * the file creation mask and restricts this value). * + * \return Zero if the path already existed as a directory or as a symbolic + * link which leads to a directory, one if the path did not exist and the + * directory has been created successfully, negative error code else. */ -static int para_opendir(const char *dirname, DIR **dir, int *cwd) +int para_mkdir(const char *path) { - int ret; + /* + * We call opendir(3) rather than relying on stat(2) because this way + * we don't need extra code to get the symlink case right. + */ + DIR *dir = opendir(path); - *dir = NULL; - if (cwd) { - ret = para_open(".", O_RDONLY, 0); - if (ret < 0) - return ret; - *cwd = ret; - } - ret = para_chdir(dirname); - if (ret < 0) - goto close_cwd; - *dir = opendir("."); - if (*dir) - return 1; - ret = -ERRNO_TO_PARA_ERROR(errno); - /* Ignore return value of fchdir() and close(). We're busted anyway. */ - if (cwd) { - int __a_unused ret2 = fchdir(*cwd); /* STFU, gcc */ + if (dir) { + closedir(dir); + return 0; } -close_cwd: - if (cwd) - close(*cwd); - return ret; -} - -/** - * A wrapper for mkdir(2). - * - * \param path Name of the directory to create. - * \param mode The permissions to use. - * - * \return Standard. - */ -int para_mkdir(const char *path, mode_t mode) -{ - if (!mkdir(path, mode)) - return 1; - return -ERRNO_TO_PARA_ERROR(errno); + if (errno != ENOENT) + return -ERRNO_TO_PARA_ERROR(errno); + return mkdir(path, 0777) == 0? 1 : -ERRNO_TO_PARA_ERROR(errno); } /** @@ -549,22 +485,21 @@ out: * \param start The start address of the memory mapping. * \param length The size of the mapping. * - * \return Standard. + * If NULL is passed as the start address, the length value is ignored and the + * function does nothing. + * + * \return Zero if NULL was passed, one if the memory area was successfully + * unmapped, a negative error code otherwise. * * \sa munmap(2), \ref mmap_full_file(). */ int para_munmap(void *start, size_t length) { - int err; - if (!start) return 0; if (munmap(start, length) >= 0) return 1; - err = errno; - PARA_ERROR_LOG("munmap (%p/%zu) failed: %s\n", start, length, - strerror(err)); - return -ERRNO_TO_PARA_ERROR(err); + return -ERRNO_TO_PARA_ERROR(errno); } /** @@ -643,64 +578,3 @@ void valid_fd_012(void) } } } - -/** - * Traverse the given directory recursively. - * - * \param dirname The directory to traverse. - * \param func The function to call for each entry. - * \param private_data Pointer to an arbitrary data structure. - * - * For each regular file under \a dirname, the supplied function \a func is - * called. The full path of the regular file and the \a private_data pointer - * are passed to \a func. Directories for which the calling process has no - * permissions to change to are silently ignored. - * - * \return Standard. - */ -int for_each_file_in_dir(const char *dirname, - int (*func)(const char *, void *), void *private_data) -{ - DIR *dir; - struct dirent *entry; - int cwd_fd, ret = para_opendir(dirname, &dir, &cwd_fd); - - if (ret < 0) - return ret == -ERRNO_TO_PARA_ERROR(EACCES)? 1 : ret; - /* scan cwd recursively */ - while ((entry = readdir(dir))) { - mode_t m; - char *tmp; - struct stat s; - - if (!strcmp(entry->d_name, ".")) - continue; - if (!strcmp(entry->d_name, "..")) - continue; - if (lstat(entry->d_name, &s) == -1) - continue; - m = s.st_mode; - if (!S_ISREG(m) && !S_ISDIR(m)) - continue; - tmp = make_message("%s/%s", dirname, entry->d_name); - if (!S_ISDIR(m)) { - ret = func(tmp, private_data); - free(tmp); - if (ret < 0) - goto out; - continue; - } - /* directory */ - ret = for_each_file_in_dir(tmp, func, private_data); - free(tmp); - if (ret < 0) - goto out; - } - ret = 1; -out: - closedir(dir); - if (fchdir(cwd_fd) < 0 && ret >= 0) - ret = -ERRNO_TO_PARA_ERROR(errno); - close(cwd_fd); - return ret; -} diff --git a/fd.h b/fd.h index 270d0ce2..e4f30903 100644 --- a/fd.h +++ b/fd.h @@ -5,14 +5,12 @@ int xrename(const char *oldpath, const char *newpath); int write_all(int fd, const char *buf, size_t len); __printf_2_3 int write_va_buffer(int fd, const char *fmt, ...); -bool file_exists(const char *); int xpoll(struct pollfd *fds, nfds_t nfds, int timeout); __must_check int mark_fd_nonblocking(int fd); __must_check int mark_fd_blocking(int fd); int para_mmap(size_t length, int prot, int flags, int fd, void *map); int para_open(const char *path, int flags, mode_t mode); -int para_mkdir(const char *path, mode_t mode); -int para_chdir(const char *path); +int para_mkdir(const char *path); int mmap_full_file(const char *filename, int open_mode, void **map, size_t *size, int *fd_ptr); int para_munmap(void *start, size_t length); @@ -21,11 +19,10 @@ int write_ok(int fd); void valid_fd_012(void); int readv_nonblock(int fd, struct iovec *iov, int iovcnt, size_t *num_bytes); int read_nonblock(int fd, void *buf, size_t sz, size_t *num_bytes); -int read_pattern(int fd, const char *pattern, size_t bufsize); +int read_and_compare(int fd, const char *expectation); int xwrite(int fd, const char *buf, size_t len); int xwritev(int fd, struct iovec *iov, int iovcnt); -int for_each_file_in_dir(const char *dirname, - int (*func)(const char *, void *), void *private_data); + /** * Write a \p NULL-terminated buffer. * diff --git a/http_recv.c b/http_recv.c index 32c9e7b9..8d2add19 100644 --- a/http_recv.c +++ b/http_recv.c @@ -105,7 +105,7 @@ static int http_recv_post_monitor(struct sched *s, void *context) return 0; } if (phd->status == HTTP_SENT_GET_REQUEST) { - ret = read_pattern(rn->fd, HTTP_OK_MSG, strlen(HTTP_OK_MSG)); + ret = read_and_compare(rn->fd, HTTP_OK_MSG); if (ret < 0) { PARA_ERROR_LOG("did not receive HTTP OK message\n"); goto out; diff --git a/http_send.c b/http_send.c index 90e3ee57..429b4662 100644 --- a/http_send.c +++ b/http_send.c @@ -170,7 +170,7 @@ static void http_post_monitor(__a_unused struct sched *s) case HTTP_STREAMING: /* nothing to do */ break; case HTTP_CONNECTED: /* need to recv get request */ - ret = read_pattern(sc->fd, HTTP_GET_MSG, MAXLINE); + ret = read_and_compare(sc->fd, HTTP_GET_MSG); if (ret < 0) phsd->status = HTTP_INVALID_GET_REQUEST; else if (ret > 0) { diff --git a/play.c b/play.c index 4024c8ea..bd183b6b 100644 --- a/play.c +++ b/play.c @@ -1048,9 +1048,9 @@ static void session_open(void) char *dot_para = make_message("%s/.paraslash", home); free(home); - ret = para_mkdir(dot_para, 0777); + ret = para_mkdir(dot_para); /* warn, but otherwise ignore mkdir error */ - if (ret < 0 && ret != -ERRNO_TO_PARA_ERROR(EEXIST)) + if (ret < 0) PARA_WARNING_LOG("Can not create %s: %s\n", dot_para, para_strerror(-ret)); history_file = make_message("%s/play.history", dot_para);