Re: svn commit: r1575428 - /subversion/trunk/subversion/libsvn_fs_fs/fs.h

On 3/7/14, 3:11 PM, Stefan Fuhrmann wrote:
> Two comments:
>
> * The keys are being used one-way, i.e. any type > input type will do

*nod* I realize it's safe. But the problem is that we know we have a type that
isn't stable for revnum's across platforms. Say we do Subversion 2.0 and we
want to fix it. If we're doing stuff like this we then have to go around and
find all the places where we used some other type to represent a revision number.

Can we at least have these private types for this sort of thing:
typedef apr_int32_t svn__revnum32_t
typedef apr_int64_t svn__revnum64_t

If we can avoid the svn__revnum32_t that'd be nice since the conversion from
svn_revnum_t to svn__revnum64_t is always safe (at least until someone shows up
with a platform with 128-bit longs), while svn_revnum_t to svn__revnum32_t is
not. Of course in practice all conversions would always be safe in our code.

With the private types it'll be easy to find these cases in the future. It
also self documents when things that aren't necessarily obviously revision numbers.

> * Padding is horrible because it inserts sections of random data
> into a *hash* key. Valgrind might find these instances, if run on Windows.

Unless you initialize the keys before filling them, but I'm guessing you want
to avoid that for performance reasons. Are you saying that Valgrind will
complain about uninitialized memory because of the padding on 64-bit Windows
(since it's going to pad due to the 64-bit word size but long is still 32-bits)?
Received on 2014-03-08 00:25:36 CET