[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1684047 - in /subversion/trunk/subversion: include/private/svn_subr_private.h libsvn_delta/svndiff.c libsvn_fs_fs/revprops.c libsvn_fs_x/revprops.c libsvn_subr/compress.c libsvn_subr/packed_data.c

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Thu, 11 Jun 2015 11:02:19 +0000

ivan_at_apache.org wrote on Sun, Jun 07, 2015 at 17:07:30 -0000:
> Author: ivan
> Date: Sun Jun 7 17:07:30 2015
> New Revision: 1684047
>
> URL: http://svn.apache.org/r1684047
> Log:
> Avoid callers of svn__compress() and svn__decompress() construct fake
> svn_stringbuf_t instance.
>
> * subversion/include/private/svn_subr_private.h
> * subversion/libsvn_subr/compress.c
> (svn__compress, svn__decompress): Replace IN parameter of type
> svn_stringbuf_t with DATA/LEN.
...
> -svn__compress(svn_stringbuf_t *in,
> +svn__compress(const void *data, apr_size_t len,
...
> -svn__decompress(svn_stringbuf_t *in,
> +svn__decompress(const void *data, apr_size_t len,
> svn_stringbuf_t *out,
> apr_size_t limit);
...
> @@ -215,19 +215,17 @@ window_handler(svn_txdelta_window_t *win
> append_encoded_int(header, window->tview_len);
> if (eb->version == 1)
> {
> - SVN_ERR(svn__compress(instructions, i1, eb->compression_level));
> + SVN_ERR(svn__compress(instructions->data, instructions->len,
> + i1, eb->compression_level));
> instructions = i1;
> }
> append_encoded_int(header, instructions->len);
> if (eb->version == 1)
> {
> svn_stringbuf_t *compressed = svn_stringbuf_create_empty(pool);
> - svn_stringbuf_t *original = svn_stringbuf_create_empty(pool);
> - original->data = (char *)window->new_data->data; /* won't be modified */
> - original->len = window->new_data->len;
> - original->blocksize = window->new_data->len + 1;
>
> - SVN_ERR(svn__compress(original, compressed, eb->compression_level));
> + SVN_ERR(svn__compress(window->new_data->data, window->new_data->len,
> + compressed, eb->compression_level));

This change introduces the possibility of bugs of the form:

    svn_error_t *foo(svn_stringbuf_t *bar, svn_string_t *baz) {
        svn_compress(bar->data, baz->len, ...); /* should be bar->len */
    }

i.e., "bar" and "baz" instead of "bar" and "bar".

I suppose having a svn_string_t formal parameter is an alternative.

It's just a minor issue, of course.

Cheers,

Daniel
Received on 2015-06-11 13:02:32 CEST

This is an archived mail posted to the Subversion Dev mailing list.