Hi Danh, On Thu, 7 Nov 2019, Danh Doan wrote: > On 2019-11-07 12:37:00 +0900, Junio C Hamano wrote: > > Danh Doan <congdanhqx@xxxxxxxxx> writes: > > > > > On 2019-11-06 11:23:00 +0900, Junio C Hamano wrote: > > >> > @@ -1454,7 +1454,7 @@ static int write_commit_graph_file(struct write_commit_graph_context *ctx) > > >> > num_chunks); > > >> > ctx->progress = start_delayed_progress( > > >> > progress_title.buf, > > >> > - num_chunks * ctx->commits.nr); > > >> > + (uint64_t)num_chunks * ctx->commits.nr); > > >> > > >> Hmph, do we need this? I understand that the second parameter to > > >> the callee is u64, so the caller needs to come up with u64 without > > >> overflow, but doesn't that automatically get promoted? > > > > > > Neither num_chunks nor ctx->commits.nr is promoted because both of > > > them are int. The result of `num_chunks * ctx->commits.nr' will be int > > > and will be promoted to u64 to pass to caller. > > > > Ah, yes. Thanks. > > > > The commit title is about "integer multiplication", but can the same > > issue arise with addition and subtraction as well, by the way? > > Yes, the same issue will arise with all binary (and ternary) arithmetic operators > (+, -, *, /, %, ^, &, |, <<, >> and ?:). > > IIRC, gcc doesn't have any warning for this kind of issue. > > Microsoft Visual Studio (2017+) has C26451 for this. > https://docs.microsoft.com/en-us/visualstudio/code-quality/c26451?view=vs-2017 > If our friends at Microsoft could help, we can check the remaining one > in our codebase. I am a bit busy right now, but it _was_ my hope that adding a Visual Studio job to our Azure Pipeline would enable everybody to perform tests like this one. In other words, I _think_ that you can add something like #pragma warning(enable: 26451) to `compat/msvc.h` and then open a PR at https://github.com/git/git, the Azure Pipeline should produce precisely what you want. (If I were you, I would also try to save some CO2 by ripping out all jobs except the `vs_build` one from `azure-pipelines.yml`.) Ciao, Dscho
