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_...

Julian Foad <julian.foad_at_wandisco.com> writes:

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

That's depends on the quality-of-implementation of the system, which
we don't control. ISO C allows NULL to be a plain 0, so to be
strictly conforming the cast in the source code is necessary.

In C++ the cast should be more common since a conforming NULL cannot
have a cast but, in the free software world at least, GCC uses
compiler magic to make plain NULL work as a pointer without a cast.

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

Redefining a system constant does not strike me as an ideal solution,
I'm not sure it's strictly valid to do it. On a practical level it
might have to be done after all the system includes in case the system
defines it unconditionally. And it should not let a cast leak out to
a C++ compiler.

-- 
Philip