> -----Original Message-----
> From: MARTIN PHILIP [mailto:codematters_at_ntlworld.com] On Behalf Of
> Philip Martin
> Sent: Saturday, June 16, 2012 1:00 AM
> To: Markus Schaber
> Cc: dev_at_subversion.apache.org
> Subject: Re: AW: [PATCH]: Optimize merge_file_trivial()
>
> Markus Schaber <m.schaber_at_3s-software.com> writes:
>
> > Hi, Philip,
> >
> > I did address most of the issues. However, the following question is
open:
> >
> > Von: Philip Martin [philip.martin_at_wandisco.com]
> >> Markus Schaber <m.schaber_at_3s-software.com> writes:
> >
> >>> +/* Function to prepare a single test file */
> >>> +
> >>> +static svn_error_t *
> >>> +create_test_file(struct test_file_definition_t* definition,
apr_pool_t
> *pool, apr_pool_t *scratch_pool)
> >>> +{
> >>> + apr_status_t status;
> >>> + apr_file_t *file_h;
> >>> + apr_off_t midpos = definition->size / 2;
> >>> + svn_error_t *err = NULL;
> >>> + int i;
> >>> +
> >>> + if (definition->size < 5)
> >>> + SVN_ERR_ASSERT(strlen(definition->data) >= definition->size);
> >>> + else
> >>> + SVN_ERR_ASSERT(strlen(definition->data) >= 5);
> >>> +
> >>> + status = apr_filepath_merge(&definition->created_path,
> >>> + TEST_DIR,
> >>> + definition->name,
> >>> + 0,
> >>> + pool);
> >
> >> Do you need to do that? Could you simply relative paths as is done
> >> subversion/tests/libsvn_diff/diff-diff3-test.c.
> >
> > I used path concatenation to pack all of the files into one temporary
> > directory, so they can be easily cleaned up, and only one svn:ignore
> > entry is needed. I did model this after how the temporary
> > repositories are handled in svn_test_fs.c.
>
> I meant use svn_relpath_join rather than apr_filepath_merge. We
> generally prefer the svn_ functions over the apr_ functions. The svn_
> functions often lead to simpler error handling because they return
> svn_error_t. The svn_ IO functions also take paths in UTF-8 which
> usually makes things easier since paths are generally UTF-8 in
> Subversion code.
This code should use svn_dirent_join() as these paths are not relpaths, but
on-disk files.
svn_relpath would assert in maintainer mode when passing a UNC or unix style
absolute path. And several required path transformations on Windows would be
ignored.
Bert
Received on 2012-06-16 07:29:32 CEST