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

Re: svn commit: r957921 - in /subversion/trunk/subversion: libsvn_wc/workqueue.c libsvn_wc/workqueue.h tests/libsvn_wc/pristine-store-test.c

From: Greg Stein <gstein_at_gmail.com>
Date: Fri, 25 Jun 2010 15:11:34 -0400

On Fri, Jun 25, 2010 at 08:36, <julianfoad_at_apache.org> wrote:
>...
> +++ subversion/trunk/subversion/libsvn_wc/workqueue.c Fri Jun 25 12:36:53 2010
>...
> +svn_error_t *
> +svn_wc__wq_build_pristine_get_translated(svn_skel_t **work_item,
> +                                         svn_wc__db_t *db,
> +                                         const char *versioned_abspath,
> +                                         const char *new_abspath,
> +                                         const svn_checksum_t *pristine_sha1,
> +                                         apr_pool_t *result_pool,
> +                                         apr_pool_t *scratch_pool)
> +{
> +  *work_item = svn_skel__make_empty_list(result_pool);
> +
> +  svn_skel__prepend_str(pristine_sha1
> +                          ? svn_checksum_serialize(pristine_sha1,
> +                                                   scratch_pool, scratch_pool)
> +                          : "",

Why is PRISTINE_SHA1 allowed to be NULL? You certainly don't
deserialize it properly in the code above.

And the serialization should go into result_pool.

>...
> +++ subversion/trunk/subversion/tests/libsvn_wc/pristine-store-test.c Fri Jun 25 12:36:53 2010
>...
> +  /* Store a pristine text, and set DATA_SHA1 and DATA_MD5. */
> +  {
> +    const char *pristine_tmp_dir;
> +    const char *pristine_tmp_abspath;
> +    svn_stream_t *pristine_tmp_stream;
> +    svn_string_t *data_string = svn_string_create(data, pool);
> +    svn_stream_t *data_stream = svn_stream_from_string(data_string, pool);
> +
> +    SVN_ERR(svn_wc__db_pristine_get_tempdir(&pristine_tmp_dir, db,
> +                                            wc_abspath, pool, pool));
> +    SVN_ERR(svn_stream_open_unique(&pristine_tmp_stream, &pristine_tmp_abspath,
> +                                   pristine_tmp_dir, svn_io_file_del_none,
> +                                   pool, pool));
> +
> +    data_stream = svn_stream_checksummed2(data_stream, &data_sha1, NULL,
> +                                          svn_checksum_sha1, TRUE, pool);
> +    data_stream = svn_stream_checksummed2(data_stream, &data_md5, NULL,
> +                                          svn_checksum_md5, TRUE, pool);
> +    SVN_ERR(svn_stream_copy3(data_stream, pristine_tmp_stream, NULL, NULL,
> +                             pool));

You could just use svn_stream_write(..., data, ...) rather than copying streams.

>...
> +  /* Check that NEW_ABSPATH has been created with the translated text. */
> +  {
> +    apr_file_t *file;
> +    char buf[1000];
> +    apr_size_t bytes_read;
> +    svn_error_t *err;
> +
> +    SVN_ERR(svn_io_file_open(&file, new_abspath,
> +                             APR_FOPEN_READ, APR_FPROT_OS_DEFAULT, pool));
> +    err = svn_io_file_read_full(file, buf, sizeof(buf), &bytes_read, pool);
> +    if (err && APR_STATUS_IS_EOF(err->apr_err))
> +      svn_error_clear(err);
> +    else
> +      SVN_ERR(err);

There is a stream/file function that sucks an entire file into an
svn_string_t or somesuch. That would be much easier to use/understand.

>...

Cheers,
-g
Received on 2010-06-25 21:12:14 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.