RE: svn commit: r1444956 - in /subversion/branches/fsfs-format7/subversion: libsvn_client/patch.c libsvn_fs_fs/fs.c libsvn_fs_fs/fs_fs.c libsvn_fs_fs/hotcopy.c libsvn_fs_fs/transaction.c libsvn_repos/repos.c libsvn_wc/adm_files.c

> -----Original Message-----
> From: stefan2_at_apache.org [mailto:stefan2_at_apache.org]
> Sent: maandag 11 februari 2013 21:47
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1444956 - in /subversion/branches/fsfs-
> format7/subversion: libsvn_client/patch.c libsvn_fs_fs/fs.c
> libsvn_fs_fs/fs_fs.c libsvn_fs_fs/hotcopy.c libsvn_fs_fs/transaction.c
> libsvn_repos/repos.c libsvn_wc/adm_files.c
>
> Author: stefan2
> Date: Mon Feb 11 20:47:10 2013
> New Revision: 1444956
>
> URL: http://svn.apache.org/r1444956
> Log:
> On the fsfs-format7 branch: update callers of the deprecated
> svn_io_file_create to svn_io_file_create2. Use the default (0)
> length value everywhere, except for those few places where the
> actual length is obvious and unlikely to change

Why did you deprecate this function instead of adding a separate optimized variant for your use case.

I don't think providing an explicit size makes sense for the average caller?

        Bert

<snip>

> Modified: subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/fs.c
> URL: http://svn.apache.org/viewvc/subversion/branches/fsfs-
> format7/subversion/libsvn_fs_fs/fs.c?rev=1444956&r1=1444955&r2=1444956
> &view=diff
> ==========================================================
> ====================
> --- subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/fs.c (original)
> +++ subversion/branches/fsfs-format7/subversion/libsvn_fs_fs/fs.c Mon
> Feb 11 20:47:10 2013
> @@ -265,8 +265,8 @@ fs_open_for_recovery(svn_fs_t *fs,
> /* Use a partly-filled fs pointer first to create 'current'. This will fail
> if 'current' already exists, but we don't care about that. */
> fs->path = apr_pstrdup(fs->pool, path);
> - svn_error_clear(svn_io_file_create(svn_fs_fs__path_current(fs, pool),
> - "0 1 1\n", pool));
> + svn_error_clear(svn_io_file_create2(svn_fs_fs__path_current(fs, pool),
> + "0 1 1\n", 0, pool));
>

Not new in this patch, but can you please try to avoid clearing *any* error when possible. Please check for the sensible errors to clear and return the rest. (There are standard macros to check for file exists, etc.)

That makes it much easier to resolve problems in the future and with the simple svn_error_compose_create() there is no good reason to drop errors.

        Bert
Received on 2013-02-11 22:04:09 CET