Add pv_asprintf() and change the way the terminal lock file is stored, so it is in a dynamically allocated buffer rather than using one of a fixed size.

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])

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 */

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

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;
+		}
 	}
 }

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) {

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;