diff --git a/configure.ac b/configure.ac index 3bdf373..503e841 100644 --- a/configure.ac +++ b/configure.ac @@ -26,7 +26,7 @@ fi dnl Items we can use if present, but can do without if not present. AC_CHECK_FUNCS([getopt_long]) -AC_CHECK_FUNCS([vsnprintf strlcat strtoul memrchr]) +AC_CHECK_FUNCS([vsnprintf vasprintf strlcat strtoul memrchr]) AC_CHECK_FUNCS([fdatasync]) AC_CHECK_FUNCS([fpathconf sysconf posix_memalign posix_fadvise]) AC_CHECK_FUNCS([nanosleep]) diff --git a/src/include/pv-internal.h b/src/include/pv-internal.h index f3db38c..c059dba 100644 --- a/src/include/pv-internal.h +++ b/src/include/pv-internal.h @@ -139,7 +139,7 @@ struct pvwatchfd_s; typedef /*@null@*/ struct pvwatchfd_s *pvwatchfd_t; /* String pointer, that is the only pointer to this resource, that can be null. */ -typedef /*@only@*/ /*@null@*/ char * nullable_string_t; +typedef /*@only@*/ /*@null@*/ char * nullable_only_string_t; /* * Structure for holding PV internal state. Opaque outside the PV library. @@ -163,7 +163,7 @@ struct pvstate_s { * Input files * ***************/ struct pvinputfiles_s { - /*@only@*/ /*@null@*/ nullable_string_t *filename; /* input filenames */ + /*@only@*/ /*@null@*/ nullable_only_string_t *filename; /* input filenames */ unsigned int file_count; /* number of input files */ } files; @@ -370,7 +370,7 @@ struct pvstate_s { * Cursor/IPC state * ********************/ struct pvcursorstate_s { - char lock_file[PV_SIZEOF_CRS_LOCK_FILE]; + /*@only@*/ /*@null@*/ char *lock_file; /* terminal lock filename */ #ifdef HAVE_IPC /*@keep@*/ /*@null@*/ struct pvipccursorstate_s *shared; /* data shared between instances */ int shmid; /* ID of our shared memory segment */ diff --git a/src/include/pv.h b/src/include/pv.h index 45fc487..06677d4 100644 --- a/src/include/pv.h +++ b/src/include/pv.h @@ -98,12 +98,27 @@ extern double pv_percentage(off_t, const off_t); * String handling wrappers. */ +/* String pointer that can be null. */ +typedef /*@null@*/ char * nullable_string_ptr; + /* * Wrapper for sprintf(), falling back to sprintf() on systems without that * function. */ extern int pv_snprintf(char *, size_t, const char *, ...); +/* + * Wrapper for snprintf(), falling back to sprintf() on systems without that + * function. + */ +extern int pv_snprintf(char *, size_t, const char *, ...); + +/* + * Wrapper for asprintf(), providing an equivalent on systems without that + * function. + */ +extern int pv_asprintf(nullable_string_ptr *, const char *, ...); + /* * Implementation of strlcat() where it is unavailable: append a string to a * buffer, constraining the buffer to a particular size and ensuring @@ -115,7 +130,7 @@ extern size_t pv_strlcat(char *, const char *, size_t); * Allocate and return a duplicate of a \0-terminated string, ensuring that * the duplicate is also \0-terminated. Returns NULL on error. */ -/*@null@ */ /*@only@ */ extern char *pv_strdup(const char *); +/*@null@ */ /*@only@ */ extern char *pv_strdup(const /*@null@ */ char *); /* * Return a pointer to the last matching character in the buffer, or NULL if diff --git a/src/pv/cursor.c b/src/pv/cursor.c index 8108621..cf06fbf 100644 --- a/src/pv/cursor.c +++ b/src/pv/cursor.c @@ -53,19 +53,17 @@ static void pv_crs_open_lockfile(pvcursorstate_t cursor, readonly_pvcontrol_t co char *tmpdir; int openflags; - /* TODO: decide whether to dynamically allocate cursor->lock_file. */ - cursor->lock_fd = -1; /* - * TODO: use ttyname_r() and dynamic buffer, or copy the result of - * ttyname() to a dynamic buffer, because basename() may modify it. + * Note that a copy is made of result of ttyname(), so it's not in a + * static buffer, because basename() may modify it. */ - ttydev = ttyname(fd); - if (!ttydev) { + ttydev = pv_strdup(ttyname(fd)); + if (NULL == ttydev) { if (!control->force) { - pv_perror("%s", _("failed to get terminal name")); + pv_error("%s", _("failed to get terminal name")); } /* * If the terminal name is unknown, then neither IPC nor a @@ -87,9 +85,20 @@ static void pv_crs_open_lockfile(pvcursorstate_t cursor, readonly_pvcontrol_t co * $TMP are rejected, and the destination buffer is bounded. */ - memset(cursor->lock_file, 0, PV_SIZEOF_CRS_LOCK_FILE); - (void) pv_snprintf(cursor->lock_file, - PV_SIZEOF_CRS_LOCK_FILE, "%s/pv-%s-%i.lock", tmpdir, basename(ttydev), (int) geteuid()); + if (NULL != cursor->lock_file) { + free(cursor->lock_file); + cursor->lock_file = NULL; + } + (void) pv_asprintf(&(cursor->lock_file), "%s/pv-%s-%i.lock", tmpdir, basename(ttydev), (int) geteuid()); + + if (NULL == cursor->lock_file) { + pv_perror("%s: %s", ttydev, _("failed to open lock file")); + free(ttydev); + cursor->disable = true; + return; + } + + free(ttydev); /* * Pawel Piatek - not everyone has O_NOFOLLOW, e.g. AIX doesn't. @@ -150,7 +159,8 @@ static void pv_crs_lock(pvcursorstate_t cursor, readonly_pvcontrol_t control, in } if (cursor->lock_fd >= 0) { - debug("%s: %s", cursor->lock_file, "terminal lock file acquired"); + debug("%s: %s", NULL == cursor->lock_file ? "(null)" : cursor->lock_file, + "terminal lock file acquired"); } else { debug("%s", "terminal lock acquired"); } @@ -178,7 +188,8 @@ static void pv_crs_unlock(pvcursorstate_t cursor, int fd) (void) fcntl(lock_fd, F_SETLK, &lock); if (cursor->lock_fd >= 0) { - debug("%s: %s", cursor->lock_file, "terminal lock file released"); + debug("%s: %s", NULL == cursor->lock_file ? "(null)" : cursor->lock_file, + "terminal lock file released"); } else { debug("%s", "terminal lock released"); } @@ -381,7 +392,10 @@ void pv_crs_init(pvcursorstate_t cursor, readonly_pvcontrol_t control, pvtransie int terminalfd; cursor->lock_fd = -2; - cursor->lock_file[0] = '\0'; + if (NULL != cursor->lock_file) { + free(cursor->lock_file); + cursor->lock_file = NULL; + } if ((!control->cursor) || (cursor->disable)) return; @@ -702,6 +716,10 @@ void pv_crs_fini(pvcursorstate_t cursor, readonly_pvcontrol_t control, pvtransie * the same time, the lock file can be removed without * further co-ordination. */ - (void) remove(cursor->lock_file); + if (NULL != cursor->lock_file) { + (void) remove(cursor->lock_file); + free(cursor->lock_file); + cursor->lock_file = NULL; + } } } diff --git a/src/pv/state.c b/src/pv/state.c index 1263b99..6302241 100644 --- a/src/pv/state.c +++ b/src/pv/state.c @@ -812,7 +812,7 @@ void pv_state_cancel_output_if_empty_format_string(pvstate_t state) void pv_state_inputfiles(pvstate_t state, unsigned int input_file_count, const char **input_files) { unsigned int file_idx; - /*@only@ */ nullable_string_t *new_array; + /*@only@ */ nullable_only_string_t *new_array; /* Free the old array and its contents, if there was one. */ if (NULL != state->files.filename) { diff --git a/src/pv/string.c b/src/pv/string.c index 1e0a0f0..648a259 100644 --- a/src/pv/string.c +++ b/src/pv/string.c @@ -22,10 +22,10 @@ /* - * Wrapper for sprintf(), falling back to sprintf() on systems without that + * Wrapper for snprintf(), falling back to sprintf() on systems without that * function. * - * Returns -1 if "str" or "format" are NULL or if "size" is 0. + * Returns -1 if "format" is NULL. * * Otherwise, ensures that the buffer "str" is always terminated with a '\0' * byte, before returning whatever the system's vsnprintf() or vsprintf() @@ -36,24 +36,26 @@ int pv_snprintf(char *str, size_t size, const char *format, ...) va_list ap; int ret; - if (NULL == str) - return -1; - if (0 == size) - return -1; - if (NULL == format) + if (NULL == format) { + errno = EINVAL; return -1; + } - str[0] = '\0'; + if (NULL != str && size > 0) + str[0] = '\0'; va_start(ap, format); + /*@-nullpass@ */ #ifdef HAVE_VSNPRINTF ret = vsnprintf(str, size, format, ap); /* flawfinder: ignore */ #else /* ! HAVE_VSNPRINTF */ ret = vsprintf(str, format, ap); /* flawfinder: ignore */ #endif /* HAVE_VSNPRINTF */ + /*@+nullpass@ *//* explicitly allowing NULL to behave as vsnprintf() does. */ va_end(ap); - str[size - 1] = '\0'; + if (NULL != str && size > 0) + str[size - 1] = '\0'; /* * flawfinder rationale: this function replaces snprintf so @@ -65,6 +67,131 @@ int pv_snprintf(char *str, size_t size, const char *format, ...) return ret; } + +/* + * Wrapper for asprintf(), working around it on systems without that + * function. + * + * Allocates a buffer large enough for the expanded format string plus a + * terminating '\0' byte, points *strp to it, and returns whatever the + * system's vasprintf() or vsprintf() returned. + * + * Returns -1 if "strp" is NULL. + * Returns -1 and sets *strp to NULL if "format" is NULL. + */ +int pv_asprintf(nullable_string_ptr *strp, const char *format, ...) +{ + va_list ap; + char *new_string = NULL; + size_t new_size = 0; + int ret; + + if (NULL == strp) + return -1; + *strp = NULL; + if (NULL == format) + return -1; + +#ifdef HAVE_VASPRINTF + va_start(ap, format); + /*@-unrecog@ */ + ret = vasprintf(strp, format, ap); /* flawfinder: ignore */ + /*@+unrecog@ *//* splint doesn't know about vasprintf(). */ + va_end(ap); + if (ret < 0) + return ret; + + new_string = *strp; + new_size = (size_t) ret + 1; +#else /* ! HAVE_VASPRINTF */ + +#ifdef HAVE_VSNPRINTF + /* Find out the required size. */ + { + char tmpbuf[8]; /* flawfinder: ignore - bounded by vsnprintf(). */ + va_start(ap, format); + ret = vsnprintf(tmpbuf, 7, format, ap); /* flawfinder: ignore */ + va_end(ap); + if (ret < 0) + return ret; + } + + /* Allocate a buffer big enough to include a terminating \0. */ + new_size = (size_t) ret + 1; + new_string = malloc(new_size); + if (NULL == new_string) + return -1; + new_string[0] = '\0'; + + /* Generate the string. */ + va_start(ap, format); + ret = vsnprintf(new_string, new_size, format, ap); /* flawfinder: ignore */ + va_end(ap); + if (ret < 0) { + int old_errno; + old_errno = errno; + free(new_string); + errno = old_errno; + return ret; + } + *strp = new_string; +#else /* ! HAVE_VSNPRINTF */ + /* + * Without vsnprintf(), determining the required size is impossible + * without a buffer to write to. The best effort here is to + * allocate a large buffer, write to it, then duplicate the string + * afterwards. This means there's an arbitrary upper bound on + * string size and there will be some heap fragmentation. + */ + + /* Allocate a large buffer for the string. */ + new_size = 16384; + new_string = malloc(new_size); + if (NULL == new_string) + return -1; + new_string[0] = '\0'; + + /* Generate the string. */ + va_start(ap, format); + ret = vsprintf(new_string, format, ap); /* flawfinder: ignore */ + va_end(ap); + if (ret < 0) { + int old_errno; + old_errno = errno; + free(new_string); + errno = old_errno; + return ret; + } + + /* Duplicate the string into a buffer just long enough for it. */ + *strp = pv_strdup(new_string); + if (NULL == *strp) { + int old_errno; + old_errno = errno; + free(new_string); + errno = old_errno; + return -1; + } + + /* Free the original large buffer, and use the new one instead. */ + free(new_string); + new_string = *strp; +#endif /* HAVE_VSNPRINTF */ +#endif /* HAVE_VASPRINTF */ + + /* Terminate the new string. */ + if (NULL != new_string && new_size > 0) + new_string[new_size - 1] = '\0'; + + /* + * flawfinder rationale: this function replaces asprintf so + * explicitly takes a non-constant format; also it explicitly + * \0-terminates the output buffer. + */ + + return ret; +} + /* * Implementation of strlcat() where it is unavailable: append a string to a * buffer, constraining the buffer to a particular size and ensuring @@ -129,7 +256,7 @@ size_t pv_strlcat(char *dst, const char *src, size_t dstsize) */ /*@null@ */ /*@only@ */ -char *pv_strdup(const char *original) +char *pv_strdup(const /*@null@ */ char *original) { size_t length; char *duplicate;