git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v5] wrapper: use a loop instead of repetitive statements

From
Jeff King <peff@peff.net>
Date
Oct 2, 2019, 15:32 UTC
Message-ID
<20191002153207.GA6116@sigill.intra.peff.net>
In-Reply-To
<xmqqbluzfzql.fsf@gitster-ct.c.googlers.com>
On Wed, Oct 02, 2019 at 03:06:10PM +0900, Junio C Hamano wrote:
Show 7 quoted lines
> All three patches looked sensible.
> 
> Even though there is no dependency or relationships among them
> (beyond that they got written at the same time by the same person),
> I'll just queue them on a single ah/cleanups topic and get them
> advance together, as I do not expect any one of them to be blocking
> the other two.

Just re-posting this follow-on from the earlier thread (it goes on top of the third one):

-- >8 --
Subject: git_mkstemps_mode(): replace magic numbers with computed value

The magic number "6" appears several times in the function, and is related to the size of the "XXXXXX" string we expect to find in the template. Let's pull that "XXXXXX" into a constant array, whose size we can get at compile time with ARRAY_SIZE().

Note that we probably can't just change this value, since callers will be feeding us a certain number of X's, but it hopefully makes the function itself easier to follow.

While we're here, let's do the same with the "letters" array (which we _could_ modify if we wanted to include more characters).

Signed-off-by: Jeff King <peff@peff.net>
---
 wrapper.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/wrapper.c b/wrapper.c
index 54541386c1..75992cff02 100644
--- a/wrapper.c
+++ b/wrapper.c
@@ -478,7 +478,9 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)
 		"abcdefghijklmnopqrstuvwxyz"
 		"ABCDEFGHIJKLMNOPQRSTUVWXYZ"
 		"0123456789";
-	static const int num_letters = 62;
+	static const int num_letters = ARRAY_SIZE(letters) - 1;
+	static const char x_pattern[] = "XXXXXX";
+	static const int num_x = ARRAY_SIZE(x_pattern) - 1;
 	uint64_t value;
 	struct timeval tv;
 	char *filename_template;
@@ -487,12 +489,12 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)
 
 	len = strlen(pattern);
 
-	if (len < 6 + suffix_len) {
+	if (len < num_x + suffix_len) {
 		errno = EINVAL;
 		return -1;
 	}
 
-	if (strncmp(&pattern[len - 6 - suffix_len], "XXXXXX", 6)) {
+	if (strncmp(&pattern[len - num_x - suffix_len], x_pattern, num_x)) {
 		errno = EINVAL;
 		return -1;
 	}
@@ -503,12 +505,12 @@ int git_mkstemps_mode(char *pattern, int suffix_len, int mode)
 	 */
 	gettimeofday(&tv, NULL);
 	value = ((uint64_t)tv.tv_usec << 16) ^ tv.tv_sec ^ getpid();
-	filename_template = &pattern[len - 6 - suffix_len];
+	filename_template = &pattern[len - num_x - suffix_len];
 	for (count = 0; count < TMP_MAX; ++count) {
 		uint64_t v = value;
 		int i;
 		/* Fill in the random bits. */
-		for (i = 0; i < 6; i++) {
+		for (i = 0; i < num_x; i++) {
 			filename_template[i] = letters[v % num_letters];
 			v /= num_letters;
 		}
-- 
2.23.0.926.g3dc922fbbc
Previous: Junio C Hamano
Message 5 of 5 in “commit-graph: remove a duplicate assignment”
  1. commit-graph: remove a duplicate assignmentAlex Henrie, Oct 1, 2019
  2. diffcore-break: use a goto instead of a redundant if statementAlex Henrie, Oct 1, 2019
  3. wrapper: use a loop instead of repetitive statementsAlex Henrie, Oct 1, 2019
  4. Junio C HamanoOct 2, 2019
  5. Jeff KingOct 2, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.