[Expat-bugs] [ expat-Patches-1437840 ] Compile fix rollup patch against 2.0.0

[SourceForge.net]

Patches item #1437840, was opened at 2006-02-23 21:58
Message generated for change (Comment added) made by kwaclaw
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=310127&aid=1437840&group_id=10127

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: None
Group: None
>Status: Open
>Resolution: None
Priority: 5
Private: No
Submitted By: Mark F. Haigh (mfhaigh)
Assigned to: Greg Stein (gstein)
Summary: Compile fix rollup patch against 2.0.0

Initial Comment:

The attached rollup patch fixes the following bugs:

#1414066 - make failes on hpux

#1033965 - Building application fails with Intel C
compiler (1.95.7)

#N/A - Build fails on Tru64 UNIX.

The patch was tested on x86-64 Linux, i386 NetBSD,
HP-UX 11i, and Tru64 5.1b.  All tests passed on each
machine.


----------------------------------------------------------------------

>Comment By: Karl Waclawek (kwaclaw)
Date: 2006-12-12 14:17

Message:
Logged In: YES 
user_id=290026
Originator: NO

I am preparing for a new release, and I came across this part of your
patch:

-ifndef INSTALL_ROOT
 INSTALL_ROOT=$(DESTDIR)
-endif

Why did you remove this logic? 
It was added due to bug # 985235 and patch # 779334.

I am inclined to add it back, though some platforms have trouble with it.
(see bug # 1490371). Maybe there is a way to improve it?

Re-opening this issue.

Karl

----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2006-03-06 09:25

Message:
Logged In: YES 
user_id=290026

Patch applied March 6, 2006.

----------------------------------------------------------------------

Comment By: Nobody/Anonymous (nobody)
Date: 2006-02-27 17:02

Message:
Logged In: NO 


You could get really anal and note that the conversion from
a pointer type to an integer type is implementation-defined
by the C standard in the first place.  But we all know it
"just works" nearly everywhere.

The code here is broken anyways.  And since the value is
never really _used_, it doesn't matter.  But changing the
cast to long prevents a warning on all architectures I know
of except for Windows-64, which uses a 32 bit long and 64
bit pointers (LLP64 model).

Using the newer C99 types is preferable, of course, and the
best thing to do is to provide an autoconf-assisted stdint.h
that can be used if the system doesn't have stdint.h.  Note
that uintptr_t and friends are optional types under C99 anyways!

But, like I said, all this does is paper-over a
(well-deserved) warning, nothing more.


----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2006-02-27 09:42

Message:
Logged In: YES 
user_id=290026

Actually, it should be uintptr_t or intptr_t, but the
question is, how many compilers recognize these types (were
they not introduced with C99?).

----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2006-02-27 09:26

Message:
Logged In: YES 
user_id=290026

One question about the patch to runtests.c:
-    int callno = 1 + (int)XML_GetUserData(parser);
+    long callno = 1 + (long)XML_GetUserData(parser);

Why changing from int to long?

If we really want to be anal, why not change to ptrdiff_t,
as callno hast to fit into a pointer?

----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2006-02-24 10:16

Message:
Logged In: YES 
user_id=290026

Thanks.
Seems to work on Cygwin as well.

Will check it in if Greg has no objections.

----------------------------------------------------------------------

Comment By: Mark F. Haigh (mfhaigh)
Date: 2006-02-23 23:10

Message:
Logged In: YES 
user_id=1458985

The patch was against the 2.0.0 release.  I'm attaching the
same patch against CVS as of Feb 23, 2006, 8:00 PST.


----------------------------------------------------------------------

Comment By: Karl Waclawek (kwaclaw)
Date: 2006-02-23 22:10

Message:
Logged In: YES 
user_id=290026

Is this patch against CVS?
It fails for me on Makefile.in:

$ patch -i compile-rollup.patch -p 1
patching file lib/expat_external.h
patching file Makefile.in
Hunk #3 succeeded at 157 (offset 1 line).
Hunk #4 FAILED at 172.
1 out of 4 hunks FAILED -- saving rejects to file 
Makefile.in.rej
patching file tests/minicheck.h
patching file tests/runtests.c


----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=310127&aid=1437840&group_id=10127