[PATCH 08 of 10] store: refactor splitting off of "data/" in _hybridencode()

[Adrian Buehlmann]

On 2012-09-17 19:58, Bryan O'Sullivan wrote:
> I've applied the previous patches in this series and pushed them to
> crew, thanks.

Thanks.

> However, this one looks wrong.
> 
>     -    res = 'data/' + '/'.join(auxencode(encodefilename(ndpath)))
>     +    res = '/'.join(auxencode(encodefilename(path)))
>          if len(res) > _maxstorepathlen:
> 
> 
> As far as I can tell, you've reduced the length of res by 5 bytes here,
> but haven't adjusted the conditional immediately afterwards to take this
> into account.

Thanks a lot for taking a close look.

Maybe I'm stubbornly overlooking something but I think it's correct what
I did.

(If it wasn't correct, then the tests wouldn't be as sharp is I thought
I made them, so this would be a second bug. Of course that's not
argument. But I then would have to improve the tests too.).

The difference is that the old line uses ndpath, whereas the new line
uses path.

ndpath was

   ndpath = path[len('data/'):]

which is path without the 'data/' prefix. The new line uses path directly.

FWIW, I saw no speed gain in encoding ndpath over the full path path.