On Fri, 2010-06-25, Greg Stein wrote:
> 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.
Oops, both copy-paste errors.
r958198.
Thanks for catching.
> >...
> > +++ 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.
I'll try those suggestions later. Thanks.
- Julian
Received on 2010-06-26 10:00:14 CEST