Subject: Re: [PATCH v3 00/13] ci: include a Visual Studio build & test in our Azure Pipeline

From: Junio C Hamano <[email-masked]>
Date: Sun, 06 Oct 2019 09:19:03 +0900

"Johannes Schindelin via GitGitGadget" <[email-masked]>
writes:

> Changes since v2:
>
>  * The overflow check introduced in v1 was consolidated into a single
>    helper.

Looks good to me.

> Range-diff vs v2:
>
>   1:  4d0b38125a =  1:  4d0b38125a push: do not pretend to return `int` from `die_push_simple()`
>   2:  8800320590 <  -:  ---------- msvc: avoid using minus operator on unsigned types
>   -:  ---------- >  2:  7fe2a85506 msvc: avoid using minus operator on unsigned types
>   3:  8512a3e96d =  3:  e632a4eef4 winansi: use FLEX_ARRAY to avoid compiler warning

This is less useful than it could be.  

With a larger creation-factor (and we can afford using a larger one,
simply because the user of GGG _knows_ that the two series being
compared are closely related), what is output is entirely readable
(attached at the end).

Oh, while I am suggesting possible improvements on GGG, can we
please tweak the sender date like git-send-email does so that two
messages in the same series do not share the same timestamp?  When
multi-patch series are displayed in MUA or public-inbox News feed
out of order, it almost always is from GGG that gave the same
timestamp to adjacent messages in a series, and it prevents me from
applying them in one go (or saving in one action to a mbox).

What send-email does is, at the beginning for N patch series, to
take the current wallclock time and subtract N seconds from it, and
then give that timestamp to the first message it sends out, and
after that, it increments the timestamp by 1 seconds.

Note that there is no need for any "sleep"---the timestamps are
given by explicitly generating the "Date: " header.  The last time
we looked into this issue, I think the code was trying to do almost
the right thing but it was giving a malformatted timezone and forcing
the sending MTA to override it with the wallclock time or something.

Thanks.

1:  9629f3c751 ! 1:  c097b95a26 msvc: avoid using minus operator on unsigned types
    @@ Commit message
         Signed-off-by: Johannes Schindelin <[email-masked]>
         Signed-off-by: Junio C Hamano <[email-masked]>
     
    + ## cache.h ##
    +@@ cache.h: struct cache_entry *index_file_exists(struct index_state *istate, const char *na
    +  */
    + int index_name_pos(const struct index_state *, const char *name, int namelen);
    + 
    ++/*
    ++ * Some functions return the negative complement of an insert position when a
    ++ * precise match was not found but a position was found where the entry would
    ++ * need to be inserted. This helper protects that logic from any integer
    ++ * underflow.
    ++ */
    ++static inline int index_pos_to_insert_pos(uintmax_t pos)
    ++{
    ++	if (pos > INT_MAX)
    ++		die("overflow: -1 - %"PRIuMAX, pos);
    ++	return -1 - (int)pos;
    ++}
    ++
    + #define ADD_CACHE_OK_TO_ADD 1		/* Ok to add */
    + #define ADD_CACHE_OK_TO_REPLACE 2	/* Ok to replace file/directory */
    + #define ADD_CACHE_SKIP_DFCHECK 4	/* Ok to skip DF conflict checks */
    +
      ## read-cache.c ##
     @@ read-cache.c: static int add_index_entry_with_check(struct index_state *istate, struct cache_e
    - 	 * we can avoid searching for it.
      	 */
      	if (istate->cache_nr > 0 &&
    --		strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
    + 		strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0)
     -		pos = -istate->cache_nr - 1;
    -+		strcmp(ce->name, istate->cache[istate->cache_nr - 1]->name) > 0) {
    -+		if (istate->cache_nr > INT_MAX)
    -+			die("overflow: -1 - %u", istate->cache_nr);
    -+		pos = -1 - (int)istate->cache_nr;
    -+	}
    ++		pos = index_pos_to_insert_pos(istate->cache_nr);
      	else
      		pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce), ce_stage(ce));
      
    @@ read-cache.c: static size_t estimate_cache_size(size_t ondisk_size, unsigned int
     
      ## sha1-lookup.c ##
     @@ sha1-lookup.c: int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
    - 			miv = take2(sha1 + ofs);
      			if (miv < lov)
      				return -1;
    --			if (hiv < miv)
    + 			if (hiv < miv)
     -				return -1 - nr;
    -+			if (hiv < miv) {
    -+				if (nr > INT_MAX)
    -+					die("overflow: -1 - %"PRIuMAX,
    -+					    (uintmax_t)nr);
    -+				return -1 - (int)nr;
    -+			}
    ++				return index_pos_to_insert_pos(nr);
      			if (lov != hiv) {
      				/*
      				 * At this point miv could be equal
    @@ sha1-lookup.c: int sha1_pos(const unsigned char *sha1, void *table, size_t nr,
      		mi = lo + (hi - lo) / 2;
      	} while (lo < hi);
     -	return -lo-1;
    -+	if (nr > INT_MAX)
    -+		die("overflow: -1 - %"PRIuMAX, (uintmax_t)lo);
    -+	return -1 - (int)lo;
    ++	return index_pos_to_insert_pos(lo);
      }
      
      int bsearch_hash(const unsigned char *sha1, const uint32_t *fanout_nbo,