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

Re: svn commit: rev 7950 - in trunk/subversion: clients/cmdline include libsvn_client libsvn_diff libsvn_fs libsvn_ra libsvn_ra_dav libsvn_ra_svn libsvn_repos libsvn_subr mod_dav_svn svnadmin svndumpfilter svnlook

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: 2003-12-09 03:00:18 CET

Erik Huelsmann wrote:
>
>>Author: ghudson
>>Date: Sun Dec 7 17:32:50 2003
>>New Revision: 7950
[...]
>> trunk/subversion/libsvn_subr/*.c
>
> which had a number of conflicts with the patch below I was going to commit.
> I was trying to maximize the use of svn_io_file_* functions to get as many
> equally formatted error strings as possible. Do you think it's a good idea to
> commit it? (It's cleaned up for the conflicts found...)

Yes, in general. Certainly the parts that reduce code and reduce the repetition of error messages are good.

> Index: subversion/libsvn_subr/config_file.c
> ===================================================================
> --- subversion/libsvn_subr/config_file.c (revision 7953)
> +++ subversion/libsvn_subr/config_file.c (working copy)
> @@ -784,10 +784,11 @@
>
> if (! apr_err)
> {
> - apr_err = apr_file_write_full (f, contents, strlen (contents), NULL);
> - if (apr_err)
> - return svn_error_wrap_apr (apr_err,
> - "Can't write config file '%s'", path);
> + SVN_ERR_W (svn_io_file_write_full (f, contents,
> + strlen (contents), NULL, pool),
> + apr_psprintf (pool,
> + "Can't write configuration file '%s'",
> + path));

But some parts, like here, are pretty ugly. I don't know if anything better can be done. It's a shame macros can't take variable argument lists.

In this case, one could argue that the "%s" part is redundant since the lower-level error will already have reported the file name, so you could simplifiy it to a constant string "Can't write configuration file", and not use apr_psprintf. I don't know whether you think that is a good idea.

I think it's OK to commit it like this if you don't want to remove the "%s" part.

- Julian

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Dec 9 02:55:03 2003

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.