You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Hyrum K. Wright" <hy...@mail.utexas.edu> on 2010/10/04 14:42:01 UTC

Re: svn commit: r1003986 [1/2] - in /subversion/trunk/subversion: libsvn_client/ libsvn_fs_base/ libsvn_fs_base/bdb/ libsvn_fs_fs/ libsvn_ra_local/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_repos/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ mo

On Mon, Oct 4, 2010 at 5:14 AM, Julian Foad <ju...@wandisco.com> wrote:
> On Mon, 2010-10-04, Philip Martin wrote:
>> stsp@apache.org writes:
>>
>> > Author: stsp
>> > Date: Sun Oct  3 16:05:19 2010
>> > New Revision: 1003986
>
> Quoting the log message:
>
>> > Starting with version 4, gcc has been printing irritating warnings about
>> > "missing sentinels in function call" for virtually every call to apr_strcat().
>
> Only with APR 1.4, and only on some platforms.
>
>> There is an issue about this in the APR bug tracker:
>> https://issues.apache.org/bugzilla/show_bug.cgi?id=47191
>>
>> The fix proposed there is to include stddef.h in apr.h, in order to make
>> sure that NULL is defined as a pointer type, such as (void *)0.
>> Unfortunately, this doesn't work for all systems, since some (e.g. OpenBSD)
>> define NULL as 0L, rather than (void *)0.
>>
>> So in spite of regrets about churn, I whipped up a sed script that changed
>> all places triggering the warning to use (char *)NULL instead of NULL as
>> sentinel, and adjusted the result to fix overlong lines and other minor
>> formatting issues. This fixes all current instances of the warning for me,
>> making it much easier to spot important warnings.  No functional change.
>
> Hi Stefan.
>
> I'm glad you are keen to keep only relevant warnings visible, but the
> sight of so many unnecessary casts in our code makes me squirm. :-(
>
> The NULL macro is intended for use as a pointer.  When a combination of
> compiler, system library headers and APR headers conspires to throw out
> lots of warnings for perfectly normal code, it is that combination that
> needs to be fixed, not our code.

I agree.  Keeping this usage consistent has the potential of being a
maintenance nightmare.  Fixing the cause, rather than the symptom,
should be the priority.

> Please could you revert this change and seek an alternative.
>
> Making 'configure' define NULL on these system as '(void *)0' should be
> a good solution until APR is fixed.

Re: svn commit: r1003986 [1/2] - in /subversion/trunk/subversion: libsvn_client/ libsvn_fs_base/ libsvn_fs_base/bdb/ libsvn_fs_fs/ libsvn_ra_local/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_repos/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ mo

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Oct 04, 2010 at 09:42:01AM -0500, Hyrum K. Wright wrote:
> On Mon, Oct 4, 2010 at 5:14 AM, Julian Foad <ju...@wandisco.com> wrote:
> > I'm glad you are keen to keep only relevant warnings visible, but the
> > sight of so many unnecessary casts in our code makes me squirm. :-(
> >
> > The NULL macro is intended for use as a pointer.  When a combination of
> > compiler, system library headers and APR headers conspires to throw out
> > lots of warnings for perfectly normal code, it is that combination that
> > needs to be fixed, not our code.
> 
> I agree.  Keeping this usage consistent has the potential of being a
> maintenance nightmare.  Fixing the cause, rather than the symptom,
> should be the priority.

OK. No time to revert it right now, will do it later tonight or tomorrow.

Stefan

Re: svn commit: r1003986 [1/2] - in /subversion/trunk/subversion: libsvn_client/ libsvn_fs_base/ libsvn_fs_base/bdb/ libsvn_fs_fs/ libsvn_ra_local/ libsvn_ra_neon/ libsvn_ra_serf/ libsvn_ra_svn/ libsvn_repos/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ mo

Posted by Stefan Sperling <st...@elego.de>.
On Mon, Oct 04, 2010 at 09:42:01AM -0500, Hyrum K. Wright wrote:
> On Mon, Oct 4, 2010 at 5:14 AM, Julian Foad <ju...@wandisco.com> wrote:
> > I'm glad you are keen to keep only relevant warnings visible, but the
> > sight of so many unnecessary casts in our code makes me squirm. :-(
> >
> > The NULL macro is intended for use as a pointer.  When a combination of
> > compiler, system library headers and APR headers conspires to throw out
> > lots of warnings for perfectly normal code, it is that combination that
> > needs to be fixed, not our code.
> 
> I agree.  Keeping this usage consistent has the potential of being a
> maintenance nightmare.  Fixing the cause, rather than the symptom,
> should be the priority.

OK. No time to revert it right now, will do it later tonight or tomorrow.

Stefan