> -----Original Message-----
> From: MARTIN PHILIP [mailto:codematt...@ntlworld.com] On Behalf Of
> Philip Martin
> Sent: Saturday, June 16, 2012 1:00 AM
> To: Markus Schaber
> Cc: dev@subversion.apache.org
> Subject: Re: AW: [PATCH]: Optimize merge_file_trivial()
> 
> Markus Schaber <m.scha...@3s-software.com> writes:
> 
> > Hi, Philip,
> >
> > I did address most of the issues. However, the following question is
open:
> >
> > Von: Philip Martin [philip.mar...@wandisco.com]
> >> Markus Schaber <m.scha...@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

Reply via email to