Re: [PATCH] xdiff: fix trivial build warnings on Windows
Excerpts from Yuya Nishihara's message of 2018-03-08 21:33:42 +0900: > On Tue, 6 Mar 2018 19:12:26 -0800, Jun Wu wrote: > > Yeah, xdiff needs a migration from using "long", "int"s to "size_t" etc. > > The git community has chosen to disallow diff >1GB files because of the > > overflow concern [1]. > > > > [1]: > > https://github.com/git/git/commit/dcd1742e56ebb944c4ff62346da4548e1e3be675 > > So, should we queue this now or leave warnings to denote things that should > be cleaned up? I think the ideal solution would be replacing all "long"s to one of: "int64_t" or "ssize_t", "size_t", instead of doing casting around. I can talk a look at the actual change, since I think I have some knowledge about xdiff internals now. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] xdiff: fix trivial build warnings on Windows
> On Mar 8, 2018, at 7:33 AM, Yuya Nishiharawrote: > >> On Tue, 6 Mar 2018 19:12:26 -0800, Jun Wu wrote: >> Yeah, xdiff needs a migration from using "long", "int"s to "size_t" etc. >> The git community has chosen to disallow diff >1GB files because of the >> overflow concern [1]. >> >> [1]: >> https://github.com/git/git/commit/dcd1742e56ebb944c4ff62346da4548e1e3be675 > > So, should we queue this now or leave warnings to denote things that should > be cleaned up? I think the subsequent work that Jun did fixed a bunch of the warnings. The only one that stood out last time I looked was the signed/unsigned comparison. This can be dropped if Jun is still working on it. I didn’t realize he was. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] xdiff: fix trivial build warnings on Windows
On Tue, 6 Mar 2018 19:12:26 -0800, Jun Wu wrote: > Yeah, xdiff needs a migration from using "long", "int"s to "size_t" etc. > The git community has chosen to disallow diff >1GB files because of the > overflow concern [1]. > > [1]: > https://github.com/git/git/commit/dcd1742e56ebb944c4ff62346da4548e1e3be675 So, should we queue this now or leave warnings to denote things that should be cleaned up? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] xdiff: fix trivial build warnings on Windows
Yeah, xdiff needs a migration from using "long", "int"s to "size_t" etc. The git community has chosen to disallow diff >1GB files because of the overflow concern [1]. [1]: https://github.com/git/git/commit/dcd1742e56ebb944c4ff62346da4548e1e3be675 Excerpts from Matt Harbison's message of 2018-03-04 17:00:11 -0500: > On Sun, 04 Mar 2018 16:45:28 -0500, Matt Harbison> wrote: > > > # HG changeset patch > > # User Matt Harbison > > # Date 1520197662 18000 > > # Sun Mar 04 16:07:42 2018 -0500 > > # Node ID c4a6b599a46f93070f5492c9e68566e6be570d2f > > # Parent 1f9bbd1d6b8ae4f7ea5d9f4310269a3b0242e7b0 > > xdiff: fix trivial build warnings on Windows > > > > These are mostly size_t to int/long conversions that are obviously safe, > > along > > with a signed/unsigned comparison. I don't have clang, so I tried > > following the > > existing whitespace convention in each module. > > The remaining xdiff warnings are: > > mercurial/thirdparty/xdiff/xmerge.c(352) : warning C4244: '=' : conversion > from '__int64' to 'long', possible loss of data > mercurial/thirdparty/xdiff/xmerge.c(355) : warning C4244: '=' : conversion > from '__int64' to 'long', possible loss of data > mercurial/thirdparty/xdiff/xutils.c(412) : warning C4244: '=' : conversion > from '__int64' to 'long', possible loss of data > mercurial/thirdparty/xdiff/xutils.c(415) : warning C4244: '=' : conversion > from '__int64' to 'long', possible loss of data > > These are a bit more concerning, because it looks like two randomish > pointers are subtracted. I haven't spent much time trying to understand > the code, so presumably there's something ensuring that these values stay > close to each other? 'long' is only 32 bit on Windows, so maybe the size > field in mmbuffer_t and mmfile_t should be size_t/ptrdiff_t? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] xdiff: fix trivial build warnings on Windows
On Sun, 04 Mar 2018 16:45:28 -0500, Matt Harbisonwrote: # HG changeset patch # User Matt Harbison # Date 1520197662 18000 # Sun Mar 04 16:07:42 2018 -0500 # Node ID c4a6b599a46f93070f5492c9e68566e6be570d2f # Parent 1f9bbd1d6b8ae4f7ea5d9f4310269a3b0242e7b0 xdiff: fix trivial build warnings on Windows These are mostly size_t to int/long conversions that are obviously safe, along with a signed/unsigned comparison. I don't have clang, so I tried following the existing whitespace convention in each module. The remaining xdiff warnings are: mercurial/thirdparty/xdiff/xmerge.c(352) : warning C4244: '=' : conversion from '__int64' to 'long', possible loss of data mercurial/thirdparty/xdiff/xmerge.c(355) : warning C4244: '=' : conversion from '__int64' to 'long', possible loss of data mercurial/thirdparty/xdiff/xutils.c(412) : warning C4244: '=' : conversion from '__int64' to 'long', possible loss of data mercurial/thirdparty/xdiff/xutils.c(415) : warning C4244: '=' : conversion from '__int64' to 'long', possible loss of data These are a bit more concerning, because it looks like two randomish pointers are subtracted. I haven't spent much time trying to understand the code, so presumably there's something ensuring that these values stay close to each other? 'long' is only 32 bit on Windows, so maybe the size field in mmbuffer_t and mmfile_t should be size_t/ptrdiff_t? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel