[PATCH v3 0/4] Fix type conversion Warings from msvc

[PATCH v3 0/4] Fix type conversion Warings from msvc

[Sören Krecker]

Hi everyone,
sorry for my late reply and thanks for your suggestions.
I am trying to improve this patch series.

Sören Krecker (4):
  add-patch: Fix type conversion warnings from msvc
  date.c: Fix type conversation warnings from msvc
  apply.c : Fix type conversation warnings from msvc
  commit.c: Fix type conversation warnings from msvc

 add-patch.c       | 53 +++++++++++++++++++++++++++--------------------
 apply.c           | 37 +++++++++++++++++----------------
 apply.h           |  6 +++---
 commit.c          | 12 +++++------
 date.c            |  8 +++----
 gettext.h         |  2 +-
 git-compat-util.h |  7 +++++++
 7 files changed, 71 insertions(+), 54 deletions(-)


base-commit: 5f8f7081f7761acdf83d0a4c6819fe3d724f01d7
-- 
2.39.5

[PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc

[Sören Krecker]

Fix some compiler warnings from msvc in add-patch.c for value truncation
form 64 bit to 32 bit integers. Change unsigned long to size_t for
correct variable size on linux and windows.
Add macro str_to_size_t for converting a string to size_t.
Test if convertion fails with over or underflow.

Signed-off-by: Sören Krecker <soekkle@freenet.de>
---
 add-patch.c       | 53 +++++++++++++++++++++++++++--------------------
 gettext.h         |  2 +-
 git-compat-util.h |  7 +++++++
 3 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/add-patch.c b/add-patch.c
index 95c67d8c80..4fb6ae2c4b 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -242,7 +242,7 @@ static struct patch_mode patch_mode_worktree_nothead = {
 };
 
 struct hunk_header {
-	unsigned long old_offset, old_count, new_offset, new_count;
+	size_t old_offset, old_count, new_offset, new_count;
 	/*
 	 * Start/end offsets to the extra text after the second `@@` in the
 	 * hunk header, e.g. the function signature. This is expected to
@@ -322,11 +322,12 @@ static void setup_child_process(struct add_p_state *s,
 }
 
 static int parse_range(const char **p,
-		       unsigned long *offset, unsigned long *count)
+		       size_t *offset, size_t *count)
 {
 	char *pend;
-
-	*offset = strtoul(*p, &pend, 10);
+	*offset = str_to_size_t(*p, &pend, 10);
+	if (errno == ERANGE)
+		return error(_("Number is too large for this field"));
 	if (pend == *p)
 		return -1;
 	if (*pend != ',') {
@@ -334,7 +335,9 @@ static int parse_range(const char **p,
 		*p = pend;
 		return 0;
 	}
-	*count = strtoul(pend + 1, (char **)p, 10);
+	*count = str_to_size_t(pend + 1, (char **)p, 10);
+	if (errno == ERANGE)
+		return error(_("Number is too large for this field"));
 	return *p == pend + 1 ? -1 : 0;
 }
 
@@ -673,8 +676,8 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
 		 */
 		const char *p;
 		size_t len;
-		unsigned long old_offset = header->old_offset;
-		unsigned long new_offset = header->new_offset;
+		size_t old_offset = header->old_offset;
+		size_t new_offset = header->new_offset;
 
 		if (!colored) {
 			p = s->plain.buf + header->extra_start;
@@ -700,12 +703,14 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
 		else
 			new_offset += delta;
 
-		strbuf_addf(out, "@@ -%lu", old_offset);
+		strbuf_addf(out, "@@ -%" PRIuMAX, (uintmax_t)old_offset);
 		if (header->old_count != 1)
-			strbuf_addf(out, ",%lu", header->old_count);
-		strbuf_addf(out, " +%lu", new_offset);
+			strbuf_addf(out, ",%" PRIuMAX,
+				    (uintmax_t)header->old_count);
+		strbuf_addf(out, " +%" PRIuMAX, (uintmax_t)new_offset);
 		if (header->new_count != 1)
-			strbuf_addf(out, ",%lu", header->new_count);
+			strbuf_addf(out, ",%" PRIuMAX,
+				    (uintmax_t)header->new_count);
 		strbuf_addstr(out, " @@");
 
 		if (len)
@@ -1066,11 +1071,13 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,
 
 	/* last hunk simply gets the rest */
 	if (header->old_offset != remaining.old_offset)
-		BUG("miscounted old_offset: %lu != %lu",
-		    header->old_offset, remaining.old_offset);
+		BUG("miscounted old_offset: %"PRIuMAX" != %"PRIuMAX,
+		    (uintmax_t)header->old_offset,
+		    (uintmax_t)remaining.old_offset);
 	if (header->new_offset != remaining.new_offset)
-		BUG("miscounted new_offset: %lu != %lu",
-		    header->new_offset, remaining.new_offset);
+		BUG("miscounted new_offset: %"PRIuMAX" != %"PRIuMAX,
+		    (uintmax_t)header->new_offset,
+		    (uintmax_t)remaining.new_offset);
 	header->old_count = remaining.old_count;
 	header->new_count = remaining.new_count;
 	hunk->end = end;
@@ -1354,9 +1361,10 @@ static void summarize_hunk(struct add_p_state *s, struct hunk *hunk,
 	struct strbuf *plain = &s->plain;
 	size_t len = out->len, i;
 
-	strbuf_addf(out, " -%lu,%lu +%lu,%lu ",
-		    header->old_offset, header->old_count,
-		    header->new_offset, header->new_count);
+	strbuf_addf(out,
+		    " -%"PRIuMAX",%"PRIuMAX" +%"PRIuMAX",%"PRIuMAX" ",
+		    (uintmax_t)header->old_offset, (uintmax_t)header->old_count,
+		    (uintmax_t)header->new_offset, (uintmax_t)header->new_count);
 	if (out->len - len < SUMMARY_HEADER_WIDTH)
 		strbuf_addchars(out, ' ',
 				SUMMARY_HEADER_WIDTH + len - out->len);
@@ -1625,10 +1633,11 @@ static int patch_update_file(struct add_p_state *s,
 			else if (0 < response && response <= file_diff->hunk_nr)
 				hunk_index = response - 1;
 			else
-				err(s, Q_("Sorry, only %d hunk available.",
-					  "Sorry, only %d hunks available.",
-					  file_diff->hunk_nr),
-				    (int)file_diff->hunk_nr);
+				err(s,
+				    Q_("Sorry, only %"PRIuMAX" hunk available.",
+				       "Sorry, only %"PRIuMAX" hunks available.",
+				       (uintmax_t)file_diff->hunk_nr),
+				    (uintmax_t)file_diff->hunk_nr);
 		} else if (s->answer.buf[0] == '/') {
 			regex_t regex;
 			int ret;
diff --git a/gettext.h b/gettext.h
index 484cafa562..d36f5a7ade 100644
--- a/gettext.h
+++ b/gettext.h
@@ -53,7 +53,7 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
 }
 
 static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
-const char *Q_(const char *msgid, const char *plu, unsigned long n)
+const char *Q_(const char *msgid, const char *plu, size_t n)
 {
 	if (!git_gettext_enabled)
 		return n == 1 ? msgid : plu;
diff --git a/git-compat-util.h b/git-compat-util.h
index e283c46c6f..bb9a6c2bc4 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -292,6 +292,13 @@ static inline int _have_unix_sockets(void)
 #include <sys/sysctl.h>
 #endif
 
+#if SIZE_MAX == ULONG_MAX
+#define str_to_size_t strtoul
+#else
+#define str_to_size_t strtoull
+#endif
+
+
 /* Used by compat/win32/path-utils.h, and more */
 static inline int is_xplatform_dir_sep(int c)
 {
-- 
2.39.5

Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc

[Patrick Steinhardt]

Note: the word after the subject's subsystem should start with a
lower-case letter.

On Sun, Jan 26, 2025 at 01:56:35PM +0100, Sören Krecker wrote:
> Fix some compiler warnings from msvc in add-patch.c for value truncation
> form 64 bit to 32 bit integers. Change unsigned long to size_t for
> correct variable size on linux and windows.
> Add macro str_to_size_t for converting a string to size_t.

There shouldn't be a need for this macro, we already have `strtoumax()`.
And in case the platform doesn't provide it we know to provide our own
implementation.

> Test if convertion fails with over or underflow.

s/convertion/conversion/

> diff --git a/add-patch.c b/add-patch.c
> index 95c67d8c80..4fb6ae2c4b 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -322,11 +322,12 @@ static void setup_child_process(struct add_p_state *s,
>  }
>  
>  static int parse_range(const char **p,
> -		       unsigned long *offset, unsigned long *count)
> +		       size_t *offset, size_t *count)
>  {
>  	char *pend;
> -
> -	*offset = strtoul(*p, &pend, 10);
> +	*offset = str_to_size_t(*p, &pend, 10);
> +	if (errno == ERANGE)
> +		return error(_("Number is too large for this field"));

Error messages should start with a lower-case letter.

>  	if (pend == *p)
>  		return -1;
>  	if (*pend != ',') {
> @@ -334,7 +335,9 @@ static int parse_range(const char **p,
>  		*p = pend;
>  		return 0;
>  	}
> -	*count = strtoul(pend + 1, (char **)p, 10);
> +	*count = str_to_size_t(pend + 1, (char **)p, 10);
> +	if (errno == ERANGE)
> +		return error(_("Number is too large for this field"));

Here, too.

> @@ -1066,11 +1071,13 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,
>  
>  	/* last hunk simply gets the rest */
>  	if (header->old_offset != remaining.old_offset)
> -		BUG("miscounted old_offset: %lu != %lu",
> -		    header->old_offset, remaining.old_offset);
> +		BUG("miscounted old_offset: %"PRIuMAX" != %"PRIuMAX,
> +		    (uintmax_t)header->old_offset,
> +		    (uintmax_t)remaining.old_offset);
>  	if (header->new_offset != remaining.new_offset)
> -		BUG("miscounted new_offset: %lu != %lu",
> -		    header->new_offset, remaining.new_offset);
> +		BUG("miscounted new_offset: %"PRIuMAX" != %"PRIuMAX,
> +		    (uintmax_t)header->new_offset,
> +		    (uintmax_t)remaining.new_offset);
>  	header->old_count = remaining.old_count;
>  	header->new_count = remaining.new_count;
>  	hunk->end = end;

I feel like most of the changes are adapting formatting directives like
this. Might be worthwhile to separate into a standalone commit. That'd
also allow the commit message to read less like a list of bullet points
and provide more context, explaining the actual change.

> diff --git a/gettext.h b/gettext.h
> index 484cafa562..d36f5a7ade 100644
> --- a/gettext.h
> +++ b/gettext.h
> @@ -53,7 +53,7 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
>  }
>  
>  static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
> -const char *Q_(const char *msgid, const char *plu, unsigned long n)
> +const char *Q_(const char *msgid, const char *plu, size_t n)
>  {
>  	if (!git_gettext_enabled)
>  		return n == 1 ? msgid : plu;

This change feels completely unrelated to all the other changes. It
would probably warrant a new commit.

> diff --git a/git-compat-util.h b/git-compat-util.h
> index e283c46c6f..bb9a6c2bc4 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -292,6 +292,13 @@ static inline int _have_unix_sockets(void)
>  #include <sys/sysctl.h>
>  #endif
>  
> +#if SIZE_MAX == ULONG_MAX
> +#define str_to_size_t strtoul
> +#else
> +#define str_to_size_t strtoull
> +#endif

Hm. A couple of comments:

  - The function name doesn't match the schema of function names we
    already have. I would rather have expected it to be called something
    like `strtouz()` or something like that.

  - We tend to avoid using `strtoul()` and friends directly, as they are
    really hard to get right. See the implementation of `strtoul_ui()`
    for all the checks we do there.

  - The way the macro is implemented feels quite fragile.

So I'd propose to adapt the approach a bit and introduce a new function
`strtoumax_ui()`:

    static inline int strtoumax_ui(char *const *s, int base, unsigned
                                   uintmax_t max, int *result);

The implementation would mostly follow what we have in `strotul_ui()`.
The `max` parameter here could be used to control the maximum that the
caller expects -- if the parsed integer exceeds it, it would return an
error and set `ERANGE`. If we had such a helper, we can then also
reimplement `strtoul_ui()` on top of that function with a simple call to
`strtoumax_ui(s, base, UINT_MAX, result)`.

This would overall be a lot more flexible than what we currently have.

Patrick

Re: [PATCH v3 0/4] Fix type conversion Warings from msvc

[Patrick Steinhardt]

On Sun, Jan 26, 2025 at 01:56:34PM +0100, Sören Krecker wrote:
> Hi everyone,
> sorry for my late reply and thanks for your suggestions.
> I am trying to improve this patch series.

Thanks for rerolling! I've got a couple more comments.

Something seems to have gone wrong when sending your patches. Only the
first patch is connected to the cover letter, the remaining ones aren't.

Patrick

Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc

[Junio C Hamano]

Patrick Steinhardt <ps@pks.im> writes:

> Note: the word after the subject's subsystem should start with a
> lower-case letter.
>
> On Sun, Jan 26, 2025 at 01:56:35PM +0100, Sören Krecker wrote:
>> Fix some compiler warnings from msvc in add-patch.c for value truncation
>> form 64 bit to 32 bit integers. Change unsigned long to size_t for
>> correct variable size on linux and windows.
>> Add macro str_to_size_t for converting a string to size_t.
>
> There shouldn't be a need for this macro, we already have `strtoumax()`.
> And in case the platform doesn't provide it we know to provide our own
> implementation.

Thanks for a detailed review; I'll omit them as I agree with all you
said there.

If I pretend for a while that moving from ulong to size_t is a good
change for line numbers and line counts in the first place, that is.

In other words, I agree with all the improvements your comments
suggest to the _implementation_.

Thanks.

Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc

[Phillip Wood]

Hi Sören

On 26/01/2025 12:56, Sören Krecker wrote:
> Fix some compiler warnings from msvc in add-patch.c for value truncation
> form 64 bit to 32 bit integers. Change unsigned long to size_t for
> correct variable size on linux and windows.

I'm afraid I'm still not convinced this is a good idea for the reasons I 
explained previously [1] together with an alternative approach to 
silencing these warnings. What makes "unsigned long" an incorrect choice 
when that's what "git diff" and "git apply" use?

[1] 
https://lore.kernel.org/git/e396131c-1bd3-46d0-bae6-cd97ca9710d8@gmail.com

> Add macro str_to_size_t for converting a string to size_t.
> Test if convertion fails with over or underflow.

That is welcome, but the implementation needs tweaking. If you look at 
other uses of strtoul() in our code you'll see that (somewhat unusually) 
one needs to set errno to zero before calling strtoul() as one cannot 
tell from the return value whether there was an error or not. As errno 
may have been set by a previous function call it needs to be cleared 
before calling strtoul() so we can be sure the error came from strtoul().

Best Wishes

Phillip

> Signed-off-by: Sören Krecker <soekkle@freenet.de>
> ---
>   add-patch.c       | 53 +++++++++++++++++++++++++++--------------------
>   gettext.h         |  2 +-
>   git-compat-util.h |  7 +++++++
>   3 files changed, 39 insertions(+), 23 deletions(-)
> 
> diff --git a/add-patch.c b/add-patch.c
> index 95c67d8c80..4fb6ae2c4b 100644
> --- a/add-patch.c
> +++ b/add-patch.c
> @@ -242,7 +242,7 @@ static struct patch_mode patch_mode_worktree_nothead = {
>   };
>   
>   struct hunk_header {
> -	unsigned long old_offset, old_count, new_offset, new_count;
> +	size_t old_offset, old_count, new_offset, new_count;
>   	/*
>   	 * Start/end offsets to the extra text after the second `@@` in the
>   	 * hunk header, e.g. the function signature. This is expected to
> @@ -322,11 +322,12 @@ static void setup_child_process(struct add_p_state *s,
>   }
>   
>   static int parse_range(const char **p,
> -		       unsigned long *offset, unsigned long *count)
> +		       size_t *offset, size_t *count)
>   {
>   	char *pend;
> -
> -	*offset = strtoul(*p, &pend, 10);
> +	*offset = str_to_size_t(*p, &pend, 10);
> +	if (errno == ERANGE)
> +		return error(_("Number is too large for this field"));
>   	if (pend == *p)
>   		return -1;
>   	if (*pend != ',') {
> @@ -334,7 +335,9 @@ static int parse_range(const char **p,
>   		*p = pend;
>   		return 0;
>   	}
> -	*count = strtoul(pend + 1, (char **)p, 10);
> +	*count = str_to_size_t(pend + 1, (char **)p, 10);
> +	if (errno == ERANGE)
> +		return error(_("Number is too large for this field"));
>   	return *p == pend + 1 ? -1 : 0;
>   }
>   
> @@ -673,8 +676,8 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
>   		 */
>   		const char *p;
>   		size_t len;
> -		unsigned long old_offset = header->old_offset;
> -		unsigned long new_offset = header->new_offset;
> +		size_t old_offset = header->old_offset;
> +		size_t new_offset = header->new_offset;
>   
>   		if (!colored) {
>   			p = s->plain.buf + header->extra_start;
> @@ -700,12 +703,14 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk,
>   		else
>   			new_offset += delta;
>   
> -		strbuf_addf(out, "@@ -%lu", old_offset);
> +		strbuf_addf(out, "@@ -%" PRIuMAX, (uintmax_t)old_offset);
>   		if (header->old_count != 1)
> -			strbuf_addf(out, ",%lu", header->old_count);
> -		strbuf_addf(out, " +%lu", new_offset);
> +			strbuf_addf(out, ",%" PRIuMAX,
> +				    (uintmax_t)header->old_count);
> +		strbuf_addf(out, " +%" PRIuMAX, (uintmax_t)new_offset);
>   		if (header->new_count != 1)
> -			strbuf_addf(out, ",%lu", header->new_count);
> +			strbuf_addf(out, ",%" PRIuMAX,
> +				    (uintmax_t)header->new_count);
>   		strbuf_addstr(out, " @@");
>   
>   		if (len)
> @@ -1066,11 +1071,13 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,
>   
>   	/* last hunk simply gets the rest */
>   	if (header->old_offset != remaining.old_offset)
> -		BUG("miscounted old_offset: %lu != %lu",
> -		    header->old_offset, remaining.old_offset);
> +		BUG("miscounted old_offset: %"PRIuMAX" != %"PRIuMAX,
> +		    (uintmax_t)header->old_offset,
> +		    (uintmax_t)remaining.old_offset);
>   	if (header->new_offset != remaining.new_offset)
> -		BUG("miscounted new_offset: %lu != %lu",
> -		    header->new_offset, remaining.new_offset);
> +		BUG("miscounted new_offset: %"PRIuMAX" != %"PRIuMAX,
> +		    (uintmax_t)header->new_offset,
> +		    (uintmax_t)remaining.new_offset);
>   	header->old_count = remaining.old_count;
>   	header->new_count = remaining.new_count;
>   	hunk->end = end;
> @@ -1354,9 +1361,10 @@ static void summarize_hunk(struct add_p_state *s, struct hunk *hunk,
>   	struct strbuf *plain = &s->plain;
>   	size_t len = out->len, i;
>   
> -	strbuf_addf(out, " -%lu,%lu +%lu,%lu ",
> -		    header->old_offset, header->old_count,
> -		    header->new_offset, header->new_count);
> +	strbuf_addf(out,
> +		    " -%"PRIuMAX",%"PRIuMAX" +%"PRIuMAX",%"PRIuMAX" ",
> +		    (uintmax_t)header->old_offset, (uintmax_t)header->old_count,
> +		    (uintmax_t)header->new_offset, (uintmax_t)header->new_count);
>   	if (out->len - len < SUMMARY_HEADER_WIDTH)
>   		strbuf_addchars(out, ' ',
>   				SUMMARY_HEADER_WIDTH + len - out->len);
> @@ -1625,10 +1633,11 @@ static int patch_update_file(struct add_p_state *s,
>   			else if (0 < response && response <= file_diff->hunk_nr)
>   				hunk_index = response - 1;
>   			else
> -				err(s, Q_("Sorry, only %d hunk available.",
> -					  "Sorry, only %d hunks available.",
> -					  file_diff->hunk_nr),
> -				    (int)file_diff->hunk_nr);
> +				err(s,
> +				    Q_("Sorry, only %"PRIuMAX" hunk available.",
> +				       "Sorry, only %"PRIuMAX" hunks available.",
> +				       (uintmax_t)file_diff->hunk_nr),
> +				    (uintmax_t)file_diff->hunk_nr);
>   		} else if (s->answer.buf[0] == '/') {
>   			regex_t regex;
>   			int ret;
> diff --git a/gettext.h b/gettext.h
> index 484cafa562..d36f5a7ade 100644
> --- a/gettext.h
> +++ b/gettext.h
> @@ -53,7 +53,7 @@ static inline FORMAT_PRESERVING(1) const char *_(const char *msgid)
>   }
>   
>   static inline FORMAT_PRESERVING(1) FORMAT_PRESERVING(2)
> -const char *Q_(const char *msgid, const char *plu, unsigned long n)
> +const char *Q_(const char *msgid, const char *plu, size_t n)
>   {
>   	if (!git_gettext_enabled)
>   		return n == 1 ? msgid : plu;
> diff --git a/git-compat-util.h b/git-compat-util.h
> index e283c46c6f..bb9a6c2bc4 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -292,6 +292,13 @@ static inline int _have_unix_sockets(void)
>   #include <sys/sysctl.h>
>   #endif
>   
> +#if SIZE_MAX == ULONG_MAX
> +#define str_to_size_t strtoul
> +#else
> +#define str_to_size_t strtoull
> +#endif
> +
> +
>   /* Used by compat/win32/path-utils.h, and more */
>   static inline int is_xplatform_dir_sep(int c)
>   {

Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> I'm afraid I'm still not convinced this is a good idea for the reasons
> I explained previously [1] together with an alternative approach to
> silencing these warnings. What makes "unsigned long" an incorrect
> choice when that's what "git diff" and "git apply" use?
>
> [1]
> https://lore.kernel.org/git/e396131c-1bd3-46d0-bae6-cd97ca9710d8@gmail.com

Ah, this patch still does that?  I was hoping that it got corrected
already after it was pointed out in the previous iterations.  I
agree with you that size_t is a dubious type to use for the line
numbers there.

diff.c defines "struct emit_callback" with lno_in_{pre,post}image
members that are of type "int", which is somewhat dubious, too, but
at least we don't run on 16-bit machines, and being limited to 2
billion lines is probably OK.  I am OK to upgrade that to long (if
we use negative values for some oob signal) or ulong, but that is
clearly outside of this topic.



>> Add macro str_to_size_t for converting a string to size_t.
>> Test if convertion fails with over or underflow.
>
> That is welcome, but the implementation needs tweaking. If you look at
> other uses of strtoul() in our code you'll see that (somewhat
> unusually) one needs to set errno to zero before calling strtoul() as
> one cannot tell from the return value whether there was an error or
> not. As errno may have been set by a previous function call it needs
> to be cleared before calling strtoul() so we can be sure the error
> came from strtoul().

Nice advice.

> Best Wishes
>
> Phillip

Thanks.


By the way, who is
<CAPig+cQ49Hdc_8=mRhhJDTny_Kqo6Wg6Nr98rsBN_YXmBrQ6kA@mail.gmail.com>
and why is such an apparently bogus e-mail address Cc'ed?

Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc

[Phillip Wood]

Hi Junio

On 29/01/2025 19:52, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
> By the way, who is
> <CAPig+cQ49Hdc_8=mRhhJDTny_Kqo6Wg6Nr98rsBN_YXmBrQ6kA@mail.gmail.com>
> and why is such an apparently bogus e-mail address Cc'ed?

That's the Reply-To address from the mail I was replying to. 
Unfortunately it does not seem to exist.

Best Wishes

Phillip

Re: [PATCH v3 1/4] add-patch: Fix type conversion warnings from msvc

[Junio C Hamano]

Phillip Wood <phillip.wood123@gmail.com> writes:

> On 29/01/2025 19:52, Junio C Hamano wrote:
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>> By the way, who is
>> <CAPig+cQ49Hdc_8=mRhhJDTny_Kqo6Wg6Nr98rsBN_YXmBrQ6kA@mail.gmail.com>
>> and why is such an apparently bogus e-mail address Cc'ed?
>
> That's the Reply-To address from the mail I was replying
> to. Unfortunately it does not seem to exist.

It just occured to me that it is probably added by a mistake and the
sender really wanted to add it to In-Reply-To: instead of Reply-To:

I wonder if this is a mistake we can do something to help users
avoid?  "git send-email" has the "--reply-to=" option and there is a
valid use case for that option, so disabling that option is a
non-starter.

Of course there are other ways to send e-mailed patches, but I do
not think of a way to misuse them with reply-to and in-reply-to
mixed up.

Thoughts?