You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Curt Arnold <ca...@apache.org> on 2005/12/20 22:50:06 UTC

Gump failures on apr-iconv and apr-util since 2005-11-10

I was looking at the Gump results for an unrelated project and  
noticed that apr-util-make and apr-iconv-make had been broken since  
2005-11-10.  The message is:

iconv_ces_euc.c: In function 'apr_iconv_euc_open':
iconv_ces_euc.c:65: error: invalid lvalue in assignment

The offending line is in /apr/apr-iconv/trunk/lib/iconv_ces_euc.c is

	CESTOSTATE(ces) = state;

The code hasn't been changed in over a year, so I'm guessing the  
failure is due to a change in the compiler on the Gump box.  I recall  
encountering problems with this line in other compilers, but I could  
not a patch in my previous bug reports.

When the CESTOSTATE macro, defined earlier in the file, is expanded  
the line becomes:

((iconv_ces_euc_state_t *)(ces)->data) = state;

Apparently earlier versions of GCC allowed the assignment to a cast  
operator, perhaps only in cases where the types match.  If the line  
is changed to:

ces->data = state;

I think all compilers would be happy, but haven't checked it myself.


Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by Curt Arnold <ca...@apache.org>.
On Dec 28, 2005, at 9:11 AM, Garrett Rooney wrote:

> I took a look at this, but the new patch appears to cause some compile
> warnings (not that this code is totally warning free in the first
> place, but I'd rather not make the problem worse).

I'm not confident with the patch either, especially since there  
didn't seem to be any unit tests around it (no "make check" as far as  
I can tell) and it would be real easy to get the operator precedence  
wrong or miscast something.  I'll mark the bug that I don't have  
confidence in the patch.



Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 12/27/05, Curt Arnold <ca...@apache.org> wrote:
>
> On Dec 22, 2005, at 5:03 PM, Garrett Rooney wrote:
>
> > On 12/22/05, Curt Arnold <ca...@apache.org> wrote:
> >
> >> I concur with the suggested change in http://issues.apache.org/
> >> bugzilla/show_bug.cgi?id=37650.  My message on it was http://
> >> marc.theaimsgroup.com/?l=apr-dev&m=113511544618645&w=2.
> >
> > Committed in r358652.  For some reason I had assumed that the problem
> > would show up in more places than just that one.
>
> Good assumption.  Neither the original reporter or I appeared to
> attempt to successfully build apr-iconv under gcc 4, but just
> analyzed the first encountered error.  There appear to be three other
> files that have a similar problem and they aren't as trivial.  I've
> take a stab at them in an attachment to bug 37650 and reopened the bug.

I took a look at this, but the new patch appears to cause some compile
warnings (not that this code is totally warning free in the first
place, but I'd rather not make the problem worse).

$ make 1>/dev/null
iconv_ces_iso2022.c: In function 'apr_iconv_iso2022_convert_to_ucs':
iconv_ces_iso2022.c:298: warning: pointer targets in assignment differ
in signedness
iconv_ccs.c: In function 'apr_iconv_ccs_event':
iconv_ccs.c:47: warning: assignment discards qualifiers from pointer
target typeucs2-internal.c: In function 'convert_from_ucs':
ucs2-internal.c:63: warning: assignment makes pointer from integer
without a cast
ucs2-internal.c: In function 'convert_to_ucs':
ucs2-internal.c:75: warning: initialization from incompatible pointer type
ucs2-internal.c:76: warning: return makes integer from pointer without a cast
ucs4-internal.c: In function 'convert_from_ucs':
ucs4-internal.c:61: warning: assignment makes pointer from integer
without a cast
ucs4-internal.c: In function 'convert_to_ucs':
ucs4-internal.c:74: warning: return makes integer from pointer without a cast
$

The warnings in ucs2-internal.c and ucs4-internal.c are new, added by
this patch.

-garrett

Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by Curt Arnold <ca...@apache.org>.
On Dec 22, 2005, at 5:03 PM, Garrett Rooney wrote:

> On 12/22/05, Curt Arnold <ca...@apache.org> wrote:
>
>> I concur with the suggested change in http://issues.apache.org/
>> bugzilla/show_bug.cgi?id=37650.  My message on it was http://
>> marc.theaimsgroup.com/?l=apr-dev&m=113511544618645&w=2.
>
> Committed in r358652.  For some reason I had assumed that the problem
> would show up in more places than just that one.

Good assumption.  Neither the original reporter or I appeared to  
attempt to successfully build apr-iconv under gcc 4, but just  
analyzed the first encountered error.  There appear to be three other  
files that have a similar problem and they aren't as trivial.  I've  
take a stab at them in an attachment to bug 37650 and reopened the bug.

Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 12/22/05, Curt Arnold <ca...@apache.org> wrote:

> I concur with the suggested change in http://issues.apache.org/
> bugzilla/show_bug.cgi?id=37650.  My message on it was http://
> marc.theaimsgroup.com/?l=apr-dev&m=113511544618645&w=2.

Committed in r358652.  For some reason I had assumed that the problem
would show up in more places than just that one.

Thanks,

-garrett

Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by Curt Arnold <ca...@apache.org>.
On Dec 22, 2005, at 12:03 PM, Garrett Rooney wrote:

> On 12/22/05, Curt Arnold <ca...@apache.org> wrote:
>
>>
>> It would still be very nice to get that one line fixed in apr-iconv
>> since apr-util is a dependency for log4cxx.   As long as apr-util is
>> not being built by Gump, neither is log4cxx and it eliminates the
>> only continuous integration system that reports our build status to
>> our list.
>
> Umm, as far as I can tell apr-iconv isn't even listed as a dependency
> for apr-util in the gump metadata, it is listed as one for
> logging-log4cxx though.  Perhaps removing the dependency from log4cxx
> would allow things to continue.  I mean it seems like apr-util should
> be picking up the system iconv anyway, since nothing in its metadata
> file indicates that it's using apr-iconv at all.

The apr-util build fails due to the same line which is involved in  
the build for both projects.  Could likely drop the apr-iconv  
dependency for log4cxx now, but would still be stopped since apr-util  
isn't building.

>
> Anyway, you have two options, either provide a patch for apr-iconv
> that fixes the problem (which I will happily review and apply), or
> alter the gump metadata so that you no longer depend on it.
>


I concur with the suggested change in http://issues.apache.org/ 
bugzilla/show_bug.cgi?id=37650.  My message on it was http:// 
marc.theaimsgroup.com/?l=apr-dev&m=113511544618645&w=2.

Index: lib/iconv_ces_euc.c
===================================================================
--- lib/iconv_ces_euc.c	(revision 358113)
+++ lib/iconv_ces_euc.c	(working copy)
@@ -62,7 +62,7 @@
	state->nccs = ces->mod->im_depcnt;
	for (i = ces->mod->im_depcnt; i; i--, depmod = depmod->im_next)
		state->ccs[i - 1] = depmod;
-	CESTOSTATE(ces) = state;
+	ces->data = state;
	return APR_SUCCESS;
}


Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 12/22/05, Curt Arnold <ca...@apache.org> wrote:

>
> It would still be very nice to get that one line fixed in apr-iconv
> since apr-util is a dependency for log4cxx.   As long as apr-util is
> not being built by Gump, neither is log4cxx and it eliminates the
> only continuous integration system that reports our build status to
> our list.

Umm, as far as I can tell apr-iconv isn't even listed as a dependency
for apr-util in the gump metadata, it is listed as one for
logging-log4cxx though.  Perhaps removing the dependency from log4cxx
would allow things to continue.  I mean it seems like apr-util should
be picking up the system iconv anyway, since nothing in its metadata
file indicates that it's using apr-iconv at all.

Or I could be missing something, I actually know very little about how
gump works.

Anyway, you have two options, either provide a patch for apr-iconv
that fixes the problem (which I will happily review and apply), or
alter the gump metadata so that you no longer depend on it.

-garrett

Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by Curt Arnold <ca...@apache.org>.
On Dec 20, 2005, at 5:19 PM, William A. Rowe, Jr. wrote:

> Garrett Rooney wrote:
>> So you want gump to build a configuration that virtually no users
>> actually use?  That seems totally useless to me.  Gump should be
>> building and testing the system as it is intended to be used, if this
>> was a win32 build, I'd be all for it testing apr-iconv, but it's a
>> unix build, it should use the support for the platform iconv just  
>> like
>> a normal user would.
>
> Granted all modern Solaris/Linux build iconv into the clib or  
> liconv as
> a system library; however users of older unicies or obscure  
> platforms still
> could certainly require some iconv support.  Once we dump iconv, if  
> we ever
> did, then I'd agree with you.
>
> Bill


It would still be very nice to get that one line fixed in apr-iconv  
since apr-util is a dependency for log4cxx.   As long as apr-util is  
not being built by Gump, neither is log4cxx and it eliminates the  
only continuous integration system that reports our build status to  
our list.

Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Garrett Rooney wrote:
> 
> So you want gump to build a configuration that virtually no users
> actually use?  That seems totally useless to me.  Gump should be
> building and testing the system as it is intended to be used, if this
> was a win32 build, I'd be all for it testing apr-iconv, but it's a
> unix build, it should use the support for the platform iconv just like
> a normal user would.

Granted all modern Solaris/Linux build iconv into the clib or liconv as
a system library; however users of older unicies or obscure platforms still
could certainly require some iconv support.  Once we dump iconv, if we ever
did, then I'd agree with you.

Bill

Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 12/20/05, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
> Garrett Rooney wrote:
> >
> > The problem started when the Gump people upgraded to a new version of
> > GCC.  There's even a bug filed about this issue, but nobody has gotten
> > motivated enough to fix it yet.  In addition to fixing the actual
> > code, it would be nice if we could list apr-iconv as an optional
> > dependency, not a required one, since on unix systems it's virtually
> > never used anyway.  I believe there is a way to make this happen, but
> > again, nobody has gotten around to changing the Gump configuration.
>
> For the purposes of gump, this seems to be a very good thing, binding to
> apr-iconv.  What the end user does, binding to apr-iconv or the clib's own
> iconv symbols, is a trivial difference.

So you want gump to build a configuration that virtually no users
actually use?  That seems totally useless to me.  Gump should be
building and testing the system as it is intended to be used, if this
was a win32 build, I'd be all for it testing apr-iconv, but it's a
unix build, it should use the support for the platform iconv just like
a normal user would.

-garrett

Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Garrett Rooney wrote:
> 
> The problem started when the Gump people upgraded to a new version of
> GCC.  There's even a bug filed about this issue, but nobody has gotten
> motivated enough to fix it yet.  In addition to fixing the actual
> code, it would be nice if we could list apr-iconv as an optional
> dependency, not a required one, since on unix systems it's virtually
> never used anyway.  I believe there is a way to make this happen, but
> again, nobody has gotten around to changing the Gump configuration.

For the purposes of gump, this seems to be a very good thing, binding to
apr-iconv.  What the end user does, binding to apr-iconv or the clib's own
iconv symbols, is a trivial difference.

Bill

Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by Curt Arnold <ca...@apache.org>.
On Dec 20, 2005, at 4:09 PM, Garrett Rooney wrote:


>
> The problem started when the Gump people upgraded to a new version of
> GCC.

As I suspected.

> There's even a bug filed about this issue, but nobody has gotten
> motivated enough to fix it yet.


Guess it is bug 37650.  I did do a Bugzilla search, but somehow  
missed it the first time.


Re: Gump failures on apr-iconv and apr-util since 2005-11-10

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 12/20/05, Curt Arnold <ca...@apache.org> wrote:

> The code hasn't been changed in over a year, so I'm guessing the
> failure is due to a change in the compiler on the Gump box.  I recall
> encountering problems with this line in other compilers, but I could
> not a patch in my previous bug reports.

The problem started when the Gump people upgraded to a new version of
GCC.  There's even a bug filed about this issue, but nobody has gotten
motivated enough to fix it yet.  In addition to fixing the actual
code, it would be nice if we could list apr-iconv as an optional
dependency, not a required one, since on unix systems it's virtually
never used anyway.  I believe there is a way to make this happen, but
again, nobody has gotten around to changing the Gump configuration.

-garrett