On Mon, Feb 9, 2015 at 4:52 PM, Evgeny Kotkov <evgeny.kot...@visualsvn.com> wrote:
> Stefan Fuhrmann <stef...@apache.org> writes: > > >> As it turns out, this particular micro-optimization makes a data leak > >> possible. This is not a real security issue, as the change happened on > >> trunk and didn't become part of any released version. Still, I think > >> that we should fix this prior to making 1.9 public. > > > > Good catch, Evgeny! Fixed in r1658439. > > I wonder if we really should be adding static const svn_string_t's, and > using > "sizeof(define) -1" for unknown performance improvements without > appropriate > measurements. I had two basic options to fix the bug. (1) Use svn_string_compare that is there for exactly that purpose. (2) Fall back to low-level C-string ops and hope to get it right this time. I chose with option (1). We use the "sizeof(define) -1" construct in at least 32 other places already. I simply went the extra mile and documented it, which makes it look like something out of the ordinary. > By the way, it looks like this changeset [1] did not really > remove the XFail() marker from the new test, so I expect the buildbots to > turn red. > Yes, I noticed that and fixed it in r1658442. -- Stefan^2.
