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/ mod_dav_...

On Mon, 2010-10-04, Philip Martin wrote:
> stsp_at_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.

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.

Thanks.

- Julian

> > * subversion/mod_authz_svn/mod_authz_svn.c
> > (get_access_conf, req_check_access, req_check_access): Use (const *)NULL
> > as sentinel for apr_pstrcat(), instead of NULL.
>
> That should be "(char *)" rather than "(const *)" in the log message.
>
> The Subversion functions svn_path_join_many and svn_dirent_join_many
> also rely on a NULL sentinel; I assume calls to these functions should
> be fixed in the same way?
Received on 2010-10-04 12:15:36 CEST