On Tue, Nov 10, 2015 at 05:34:45PM -0800, Bruce Dawson wrote: > Sure. I'll do that.
okay, thanks > On old architectures it would be easy enough to use a typedef to 'create' > intptr_t, so any compatibility problems that such a patch introduces will > be easily fixed. the problem is to know when that type is not defined, will require some configure.ac magic I'm afraid Daniel > On Tue, Nov 10, 2015 at 5:32 PM, Daniel Veillard <veill...@redhat.com> > wrote: > > > On Tue, Nov 10, 2015 at 01:04:51PM -0800, Bruce Dawson wrote: > > > Resending now that I've joined the mailing list... > > > > > > While building 64-bit Chromium with VC++ 2015 Update 1 I noticed a > > > significant number of pointer truncation warnings in libxml, especially > > in > > > xpath.c. A typical warning is: > > > > > > warning C4311: 'type cast': pointer truncation from 'xmlChar *' to 'long' > > > > > > which triggers on the last two lines of this block: > > > > > > case XML_ELEMENT_NODE: > > > if (node2->type == XML_ELEMENT_NODE) { > > > if ((0 > (long) node1->content) && /* TODO: Would a != 0 suffice here? */ > > > (0 > (long) node2->content) && > > > > > > The intent is not entirely clear but if these are supposed to be NULL > > > checks then they could easily give the wrong result. In the VC++ world > > > 'long' is always a 32-bit type, so converting a pointer to a long both > > > throws away the top 32 bits and also makes it a signed type. That means > > > that pointers to the first 2 GB of address space will behave as expected, > > > and pointers beyond that will have a 50% chance of behaving incorrectly! > > > > > > Another example is these two conversions. The code is apparently trying > > to > > > sort by content address, but on 64-bit Windows builds it will just be > > > sorting by the bottom 32 bits of the content address. > > > l1 = -((long) node1->content); > > > l2 = -((long) node2->content); > > > if (l1 < l2) > > > return(1); > > > if (l1 > l2) > > > return(-1); > > > } > > > > > > This is not a problem with gcc or clang because their 64-bit builds have > > > 64-bit long, but the C/C++ standard do not mandate and that is not the > > case > > > with VC++. > > > > > > I think that the correct type would be ptrdiff_t, or size_t, or intptr_t, > > > or uintptr_t. I've attached the full set of warnings in case that is of > > any > > > assistance. Even though some of these warnings do not indicate actual > > bugs > > > it would still be best to use the correct type in order to avoid > > confusion > > > and warnings. > > > > > > > Sounds like intptr_t would be the proper type to use for those, I just > > wonder what kind of old arches will break if they don't have it. > > > > Can you come up with a patch which just change the casts and possibly the > > target > > types ? > > > > Daniel > > > > -- > > Daniel Veillard | Open Source and Standards, Red Hat > > veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ > > http://veillard.com/ | virtualization library http://libvirt.org/ > > > > > > -- > Bruce Dawson -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ _______________________________________________ xml mailing list, project page http://xmlsoft.org/ xml@gnome.org https://mail.gnome.org/mailman/listinfo/xml