RE: svn commit: r1535399 - in /subversion/trunk/subversion: libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/ tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/ tests/libsvn_subr/ tests/libsvn_wc/

> -----Original Message-----
> From: Philip Martin [mailto:philip.martin_at_wandisco.com]
> Sent: vrijdag 25 oktober 2013 11:41
> To: Branko Čibej
> Cc: Subversion Development
> Subject: Re: svn commit: r1535399 - in /subversion/trunk/subversion:
> libsvn_fs_fs/ libsvn_fs_x/ libsvn_subr/ libsvn_wc/ mod_authz_svn/
> tests/cmdline/ tests/libsvn_client/ tests/libsvn_fs_fs/ tests/libsvn_fs_x/
> tests/libsvn_subr/ tests/libsvn_wc/
>
> Branko Čibej <brane_at_wandisco.com> writes:
>
> > Sideline: *IF* we, as a community, agree to do this, then: (a) the rule
> > should be documented in HACKING, and (b) there should be a '#define
> > NULLPTR ((void*)0)' or reasonable facsimile in either svn_types.h or
> > svn_private_config.h. I really do not want to see '(char*)NULL' all over
> > the place. That's ... unsightly.
>
> That would be incompatible with C++ since (void*) doesn't automatically
> convert to (char*) by default. I'd like to avoid such incompatibilities
> because I'd like to be able to use a C++ compiler at some point to catch
> things like enum mismatches. I suppose we could have NULLCHARPTR for
> variadic functions expecting (char*) which is the common case.

Our C++ code can just use NULL as it does today, while our C code uses a NULLPTR defined in a private header.

I agree that I don't like the casts there. I don't think any of the supported architectures has a problem with the original code.

On Windows where int and long are always 4 byte in x64 mode, the varargs on x64 always use 8 bytes to transfer integer types to avoid the problems we are talking about here. So as far as I can tell we are only looking at some gcc warning that we somehow enable, and want to suppress.

In the swig bindings there is a far more serious problem where we convert function pointers to void* and back, which is illegal according to the spec. (Gives hundreds if not thousands of warnings on the Windows build). If we really want to spend time on these issues we should look at those instead.

Luckily the same thing applies here: I don't know of currently supported architectures that care about this problem.

        Bert
Received on 2013-10-25 11:48:59 CEST