Re: [PATCH] xdiff: fix trivial build warnings on Windows

2018-03-09 Thread Jun Wu 
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

2018-03-08 Thread Matt Harbison 

> On Mar 8, 2018, at 7:33 AM, Yuya Nishihara  wrote:
> 
>> 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

2018-03-08 Thread Yuya Nishihara 
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

2018-03-06 Thread Jun Wu 
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

2018-03-04 Thread Matt Harbison 
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

[PATCH] xdiff: fix trivial build warnings on Windows

2018-03-04 Thread Matt Harbison 
# 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.

diff --git a/mercurial/thirdparty/xdiff/xemit.c 
b/mercurial/thirdparty/xdiff/xemit.c
--- a/mercurial/thirdparty/xdiff/xemit.c
+++ b/mercurial/thirdparty/xdiff/xemit.c
@@ -31,7 +31,7 @@
 
 
 static int xdl_emit_record(xdfile_t *xdf, long ri, char const *pre, xdemitcb_t 
*ecb) {
-   long size, psize = strlen(pre);
+   long size, psize = (long)strlen(pre);
char const *rec;
 
size = xdl_get_rec(xdf, ri, );
@@ -81,7 +81,8 @@
} else if (distance < max_ignorable && xch->ignore) {
ignored += xch->chg2;
} else if (lxch != xchp &&
-  xch->i1 + ignored - (lxch->i1 + lxch->chg1) > 
max_common) {
+  xch->i1 + ignored - (lxch->i1 + lxch->chg1)
+   > (unsigned long)max_common) {
break;
} else if (!xch->ignore) {
lxch = xch;
diff --git a/mercurial/thirdparty/xdiff/xmerge.c 
b/mercurial/thirdparty/xdiff/xmerge.c
--- a/mercurial/thirdparty/xdiff/xmerge.c
+++ b/mercurial/thirdparty/xdiff/xmerge.c
@@ -199,9 +199,9 @@
  int size, int i, int style,
  xdmerge_t *m, char *dest, int marker_size)
 {
-   int marker1_size = (name1 ? strlen(name1) + 1 : 0);
-   int marker2_size = (name2 ? strlen(name2) + 1 : 0);
-   int marker3_size = (name3 ? strlen(name3) + 1 : 0);
+   int marker1_size = (name1 ? (int)strlen(name1) + 1 : 0);
+   int marker2_size = (name2 ? (int)strlen(name2) + 1 : 0);
+   int marker3_size = (name3 ? (int)strlen(name3) + 1 : 0);
int needs_cr = is_cr_needed(xe1, xe2, m);
 
if (marker_size <= 0)
diff --git a/mercurial/thirdparty/xdiff/xutils.c 
b/mercurial/thirdparty/xdiff/xutils.c
--- a/mercurial/thirdparty/xdiff/xutils.c
+++ b/mercurial/thirdparty/xdiff/xutils.c
@@ -51,7 +51,7 @@
mb[1].size = size;
if (size > 0 && rec[size - 1] != '\n') {
mb[2].ptr = (char *) "\n\\ No newline at end of file\n";
-   mb[2].size = strlen(mb[2].ptr);
+   mb[2].size = (long) strlen(mb[2].ptr);
i++;
}
if (ecb->outf(ecb->priv, mb, i) < 0) {
@@ -341,7 +341,7 @@
*str++ = '0';
*str = '\0';
 
-   return str - out;
+   return (int) (str - out);
 }
 
 int xdl_emit_hunk_hdr(long s1, long c1, long s2, long c2,
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel