On 8/7/14 4:10 AM, Ivan Zhakov wrote:
> Several comments on branch code itself:
> 1. Probably it makes sense to do not deprecate
> svn_checksum_to_cstring_display() or have local x509 implementation
> for fingerprint formatting because we use
> svn_checksum_to_cstring_display() as canonical representation of
> checksum in libsvn_fs/libsvn_repos. So I suggest revert r1616093 from
> branch and commit attached patch.
>
> This avoid a lot of unrelated changes. I'm ready to do this. Do you
> have any objections?
The biggest reason I went ahead and deprecated the old function is to try and
drive us to think about what format we're outputting. Quite a few of the
places we're outputting the rather difficult to read and compare format to end
users. We should probably change that but I think it's well outside the scope
of the branch so I didn't do it.
As you point out there are also a number of places where the formatting is part
of our format. It's not just the repository, but it's also used in the
filenames for pristines.
I think I would much rather have these sorts of uses move to some other
function name (or maybe a macro) since frankly
svn_checksum_to_cstring_display() seems like the wrong name for a function
that's outputting as you put it "canonical representations of checksums", which
are not ended to be displayed.
I certainly was surprised by all the places we ended up using it and felt that
someone could have easily broken the repository by making a change to the
formatting.
I'm not fond of the idea of making that format private to the auth command
because then 3rd parties have to duplicate it to match our output and we can't
use it in other places that I think we probably ought to be using.
> 2. There are several new compilation warnings on Windows using VS2013:
> ..\..\..\subversion\libsvn_subr\x509parse.c(101): warning C4018: '>' :
> signed/unsigned mismatch
> ..\..\..\subversion\libsvn_subr\x509parse.c(1054): warning C4389: '!='
> : signed/unsigned mismatch
This appears to be because pointers are unsigned and apr_size_t is signed.
Guess we can just cast the pointer arithmetic to apr_size_t. So they end up
looking like this respectively:
if (*len > (apr_size_t)(end - *p))
if (len != (apr_size_t)(end - p))
> ..\..\..\subversion\libsvn_subr\x509parse.c(939): warning C4018: '<' :
> signed/unsigned mismatch
> ..\..\..\subversion\libsvn_subr\x509parse.c(946): warning C4389: '!='
> : signed/unsigned mismatch
This is an easy fix. int i should be apr_size_t i;
Received on 2014-08-07 19:06:19 CEST