Re: [PATCH 1/4] add-patch: Fix type missmatch rom msvc

[Phillip Wood <phillip.wood123@gmail.com>]

On 26/12/2024 21:33, Junio C Hamano wrote:
> Sören Krecker <soekkle@freenet.de> writes:
> 
>> Fix some compiler warings from msvw 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
>>
>> Signed-off-by: Sören Krecker <soekkle@freenet.de>
>> ---
>>   add-patch.c | 44 +++++++++++++++++++++++++-------------------
>>   gettext.h   |  2 +-
>>   2 files changed, 26 insertions(+), 20 deletions(-)
> 
> 
> 
>>   struct hunk_header {
>> -	unsigned long old_offset, old_count, new_offset, new_count;
>> +	size_t old_offset, old_count, new_offset, new_count;
> 
> These are not "size"s in the traditional sense of what size_t is
> (i.e. the number of bytes in a region of memory), but are more or
> less proportional to that in that they count in number of lines.
> 
> If ulong is sufficient to count number of lines in an incoming
> patch, then turning size_t may be excessive---are we sure that we
> are not unnecessarily using wider-than-necessary size_t in some
> places to hold these values for which ulong is sufficient, causing
> compilers to emit unnecessary warning?

That's my thought too - I think something like the diff below should
fix the warnings by using more appropriate types in expressions
involving the hunk header offset and count. Our internal diff
implementation will not generate diffs for blobs greater than ~1GB
and I don't think "git apply" can handle diff headers that contain
numbers greater that ULONG_MAX so switching to size_t here seems
unnecessary.

Best Wishes

Phillip

---- >8 ----

diff --git a/add-patch.c b/add-patch.c
index 557903310de..2c439b83665 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -253,7 +253,7 @@ struct hunk_header {
  
  struct hunk {
  	size_t start, end, colored_start, colored_end, splittable_into;
-	ssize_t delta;
+	long delta;
  	enum { UNDECIDED_HUNK = 0, SKIP_HUNK, USE_HUNK } use;
  	struct hunk_header header;
  };
@@ -760,7 +760,8 @@ static void render_diff_header(struct add_p_state *s,
  static int merge_hunks(struct add_p_state *s, struct file_diff *file_diff,
  		       size_t *hunk_index, int use_all, struct hunk *merged)
  {
-	size_t i = *hunk_index, delta;
+	size_t i = *hunk_index;
+	long delta;
  	struct hunk *hunk = file_diff->hunk + i;
  	/* `header` corresponds to the merged hunk */
  	struct hunk_header *header = &merged->header, *next;
@@ -890,7 +891,7 @@ static void reassemble_patch(struct add_p_state *s,
  {
  	struct hunk *hunk;
  	size_t save_len = s->plain.len, i;
-	ssize_t delta = 0;
+	long delta = 0;
  
  	render_diff_header(s, file_diff, 0, out);
  
@@ -926,7 +927,8 @@ static int split_hunk(struct add_p_state *s, struct file_diff *file_diff,
  	int colored = !!s->colored.len, first = 1;
  	struct hunk *hunk = file_diff->hunk + hunk_index;
  	size_t splittable_into;
-	size_t end, colored_end, current, colored_current = 0, context_line_count;
+	size_t end, colored_end, current, colored_current = 0;
+	unsigned long context_line_count;
  	struct hunk_header remaining, *header;
  	char marker, ch;
  
@@ -1175,8 +1177,8 @@ static int edit_hunk_manually(struct add_p_state *s, struct hunk *hunk)
  	return 1;
  }
  
-static ssize_t recount_edited_hunk(struct add_p_state *s, struct hunk *hunk,
-				   size_t orig_old_count, size_t orig_new_count)
+static long recount_edited_hunk(struct add_p_state *s, struct hunk *hunk,
+				 unsigned long orig_old_count, unsigned long orig_new_count)
  {
  	struct hunk_header *header = &hunk->header;
  	size_t i;
@@ -1626,7 +1628,7 @@ static int patch_update_file(struct add_p_state *s,
  			else
  				err(s, Q_("Sorry, only %d hunk available.",
  					  "Sorry, only %d hunks available.",
-					  file_diff->hunk_nr),
+					  (int)file_diff->hunk_nr),
  				    (int)file_diff->hunk_nr);
  		} else if (s->answer.buf[0] == '/') {
  			regex_t regex;