From e1d68f58ab6a2fe43eb011555ab9a1582d523bab Mon Sep 17 00:00:00 2001
From: Kp <kp@valhallalegends.com>
Date: Sun, 7 Jan 2024 19:01:49 +0000
Subject: [PATCH] Rework file browser to quiet spurious gcc-14 string warning

gcc-14 warns:

```
similar/main/menu.cpp: In member function 'virtual dcx::window_event_result dcx::{anonymous}::browser::callback_handler(const dcx::d_event&, dcx::window_event_result)':
similar/main/menu.cpp:2349:86: error: '%s' directive output may be truncated writing up to 4095 bytes into a region of size between 1 and 4096 [-Werror=format-truncation=]
 2349 |                     snprintf(&newpath[len_newpath], newpath.size() - len_newpath, "%s%s", (len_newpath >= len_sep && strncmp(&newpath[len_newpath - len_sep], sep, len_sep)) ? sep : "", list[citem]);
      |                                                                                      ^~
```

This is because it assumes printing `sep` could consume the entire
buffer, leaving no space for `list[citem]`.  It can be silenced by using
`%.*s` for both strings, and setting the length limit equal to
`strlen(that_string)`, which is an expensive way of writing the same
expression.  `snprintf` will already stop at the null terminator, so the
length limit does not constrain it further.  The warning might be
explained if the compiler assumes that the strings became longer after
they were first measured, but such a change cannot happen in this
program.

Avoid the whole problem by replacing the `snprintf` with a pair of
`std::ranges::copy_n` calls, which implementations should be able to
convert to `memcpy`.
---
 similar/main/menu.cpp | 37 ++++++++++++++++++++++++-------------
 1 file changed, 24 insertions(+), 13 deletions(-)

diff --git a/similar/main/menu.cpp b/similar/main/menu.cpp
index c372b32e3..0be35063e 100644
--- a/similar/main/menu.cpp
+++ b/similar/main/menu.cpp
@@ -2332,21 +2332,32 @@ window_event_result browser::callback_handler(const d_event &event, window_event
 				return get_absolute_path(userdata, "");
 			else
 			{
-				const size_t len_newpath = strlen(newpath.data());
-				const size_t len_item = strlen(list[citem]);
-				/* The separator may or may not be included, depending on
-				 * whether one is already present.  Regardless of whether a new
-				 * separator is printed by the block, require sufficient space
-				 * for it to fit.  This is pessimistic, but should discourage
-				 * gcc's -Wformat-truncation from warning about insufficient
-				 * buffer capacity.  In practice, users should never have a
-				 * path that approaches this limit, so the `if` test should
-				 * succeed in all reasonable cases.
+				const std::size_t len_newpath{strlen(newpath.data())};
+				const std::size_t len_item{1 + strlen(list[citem])};	// also copy the trailing null
+				const std::size_t len_sep{strlen(sep)};
+				/* If `newpath` is long enough to contain a separator, test
+				 * whether it ends with a separator.  If it is not long enough
+				 * to contain one, or is tested and does not end with one, then
+				 * set `len_copy_sep` to copy from `sep`.  Otherwise, set
+				 * `len_copy_sep` to copy nothing.
 				 */
-				const size_t len_sep = strlen(sep);
-				if (len_newpath + len_item + len_sep < newpath.size())
+				const std::size_t len_copy_sep{
+					(len_newpath >= len_sep && !memcmp(&newpath[len_newpath - len_sep], sep, len_sep))
+						? std::size_t{0}
+						: len_sep
+				};
+				const std::size_t capacity_newpath{newpath.size()};
+				/* In practice, users should never have a path that approaches
+				 * this limit, so the `if` test should succeed in all
+				 * reasonable cases.
+				 */
+				if (const std::size_t total_used_after_insertion{len_newpath + len_copy_sep + len_item}; total_used_after_insertion < capacity_newpath)
 				{
-					snprintf(&newpath[len_newpath], newpath.size() - len_newpath, "%s%s", (len_newpath >= len_sep && strncmp(&newpath[len_newpath - len_sep], sep, len_sep)) ? sep : "", list[citem]);
+					const auto begin_insert_sep{std::next(newpath.begin(), len_newpath)};
+					const auto begin_insert_item{std::ranges::copy_n(sep, len_copy_sep, begin_insert_sep).out};
+					const auto end_insert_item{std::ranges::copy_n(list[citem], len_item, begin_insert_item).out};
+					(void)end_insert_item;
+					assert(end_insert_item == std::next(newpath.begin(), total_used_after_insertion));
 				}
 			}
 			if ((citem == 0) || PHYSFS_isDirectory(list[citem]))