You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <e....@gmx.net> on 2003/12/08 17:14:58 UTC
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
> Author: ghudson
> Date: Sun Dec 7 17:32:50 2003
> New Revision: 7950
>
> Modified:
[...]
> 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...)
Unfinished log:
[[[
make libsvn_subr use the svn_io_file_* interface as much as possible.
Also implement the svn_io_file_info_get wrapper around apr_file_info_get as
needed for libsvn_diff.
]]]
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));
SVN_ERR (svn_io_file_close (f, pool));
}
@@ -891,10 +892,10 @@
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));
SVN_ERR (svn_io_file_close (f, pool));
}
@@ -1019,10 +1020,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));
SVN_ERR (svn_io_file_close (f, pool));
}
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 7953)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -349,18 +349,11 @@
apr_file_t *s;
apr_finfo_t finfo;
- apr_err = apr_file_open (&s, src_apr, APR_READ, APR_OS_DEFAULT,
pool);
- if (apr_err)
- return svn_error_wrap_apr (apr_err, "Can't open '%s' for perms",
src);
+ SVN_ERR_W (svn_io_file_open (&s, src_apr,
+ APR_READ, APR_OS_DEFAULT, pool),
+ apr_psprintf (pool,"Can't open '%s' for permissions",
src));
- apr_err = apr_file_info_get (&finfo, APR_FINFO_PROT, s);
- if (apr_err)
- {
- apr_file_close (s); /* toss any error */
- return svn_error_wrap_apr
- (apr_err, "Can't get perm info for '%s'", src);
- }
-
+ SVN_ERR (svn_io_file_info_get (&finfo, APR_FINFO_PROT, s));
SVN_ERR (svn_io_file_close (s, pool));
apr_err = apr_file_perms_set (dst_tmp_apr, finfo.protection);
@@ -559,7 +552,6 @@
const char *contents,
apr_pool_t *pool)
{
- apr_status_t apr_err;
apr_file_t *f;
apr_size_t written;
@@ -568,10 +560,8 @@
APR_OS_DEFAULT,
pool));
- apr_err = apr_file_write_full (f, contents, strlen (contents), &written);
- if (apr_err)
- return svn_error_wrap_apr (apr_err, "Can't write '%s'", file);
-
+ SVN_ERR (svn_io_file_write_full (f, contents, strlen (contents),
+ &written, pool));
SVN_ERR (svn_io_file_close (f, pool));
return SVN_NO_ERROR;
@@ -682,8 +672,9 @@
{
struct apr_md5_ctx_t context;
apr_file_t *f = NULL;
- apr_status_t apr_err;
+ svn_error_t *err;
char buf[BUFSIZ]; /* What's a good size for a read chunk? */
+ apr_size_t len;
/* ### The apr_md5 functions return apr_status_t, but they only
return success, and really, what could go wrong? So below, we
@@ -693,18 +684,19 @@
SVN_ERR (svn_io_file_open (&f, file, APR_READ, APR_OS_DEFAULT, pool));
- do {
- apr_size_t len = BUFSIZ;
+ len = sizeof (buf);
+ err = svn_io_file_read (f, buf, &len, pool);
+ while (! err)
+ {
+ apr_md5_update (&context, buf, len);
+ len = sizeof (buf);
+ err = svn_io_file_read (f, buf, &len, pool);
+ };
+
+ if (err && ! APR_STATUS_IS_EOF(err->apr_err))
+ return err;
+ svn_error_clear (err);
- apr_err = apr_file_read (f, buf, &len);
-
- if (apr_err && ! APR_STATUS_IS_EOF(apr_err))
- return svn_error_wrap_apr (apr_err, "Can't read from '%s'", file);
-
- apr_md5_update (&context, buf, len);
-
- } while (! APR_STATUS_IS_EOF(apr_err));
-
SVN_ERR (svn_io_file_close (f, pool));
apr_md5_final (digest, &context);
@@ -952,7 +944,7 @@
apr_pool_t *pool)
{
apr_size_t len;
- apr_status_t apr_err;
+ svn_error_t *err;
svn_stringbuf_t *res = svn_stringbuf_create("", pool);
char buf[BUFSIZ];
@@ -961,16 +953,16 @@
/* apr_file_read will not return data and eof in the same call. So this
loop
* is safe from missing read data. */
len = sizeof(buf);
- apr_err = apr_file_read (file, buf, &len);
- while (! apr_err)
+ err = svn_io_file_read (file, buf, &len, pool);
+ while (! err)
{
svn_stringbuf_appendbytes(res, buf, len);
len = sizeof(buf);
- apr_err = apr_file_read (file, buf, &len);
+ err = svn_io_file_read (file, buf, &len, pool);
}
/* Having read all the data we *expect* EOF */
- if (!APR_STATUS_IS_EOF(apr_err))
+ if (err && !APR_STATUS_IS_EOF(err->apr_err))
{
const char *fname_utf8;
SVN_ERR (file_name_get (&fname_utf8, file, pool));
@@ -981,7 +973,11 @@
if (NULL == fname_utf8)
fname_utf8 = "(stdin)";
- return svn_error_wrap_apr (apr_err, "Can't read from '%s'",
fname_utf8);
+ return
+ svn_error_quick_wrap (err,
+ apr_psprintf (pool,
+ "EOF not seen for '%s'",
+ fname_utf8));
}
/* Null terminate the stringbuf. */
@@ -1524,7 +1520,7 @@
svn_node_kind_t kind;
apr_file_t *fh;
- apr_status_t apr_err;
+ svn_error_t *err;
unsigned char block[1024];
apr_size_t amt_read = sizeof (block);
@@ -1542,12 +1538,13 @@
SVN_ERR (svn_io_file_open (&fh, file, APR_READ, 0, pool));
/* Read a block of data from FILE. */
- apr_err = apr_file_read (fh, block, &amt_read);
- if (apr_err && ! APR_STATUS_IS_EOF(apr_err))
- return svn_error_wrap_apr (apr_err, "Can't read '%s'", file);
+ err = svn_io_file_read (fh, block, &amt_read, pool);
+ if (err && ! APR_STATUS_IS_EOF(err->apr_err))
+ return err;
+ svn_error_clear (err);
/* Now close the file. No use keeping it open any more. */
- apr_file_close (fh);
+ SVN_ERR (svn_io_file_close (fh, pool));
/* Right now, this function is going to be really stupid. It's
@@ -1625,6 +1622,7 @@
return svn_error_wrap_apr (status, "Can't %s %s", op, name);
}
+
svn_error_t *
svn_io_file_close (apr_file_t *file, apr_pool_t *pool)
{
@@ -1644,6 +1642,16 @@
svn_error_t *
+svn_io_file_info_get (apr_finfo_t *finfo, apr_int32_t wanted,
+ apr_file_t *file, apr_pool_t *pool)
+{
+ return do_io_file_wrapper_cleanup
+ (file, apr_file_info_get (finfo, wanted, file),
+ "get file information from", pool);
+}
+
+
+svn_error_t *
svn_io_file_read (apr_file_t *file, void *buf,
apr_size_t *nbytes, apr_pool_t *pool)
{
@@ -1665,6 +1673,16 @@
svn_error_t *
+svn_io_file_seek (apr_file_t *file, apr_seek_where_t where,
+ apr_off_t *offset, apr_pool_t *pool)
+{
+ return do_io_file_wrapper_cleanup
+ (file, apr_file_seek (file, where, offset),
+ "set file pointer in", pool);
+}
+
+
+svn_error_t *
svn_io_file_write (apr_file_t *file, const void *buf,
apr_size_t *nbytes, apr_pool_t *pool)
{
@@ -2100,7 +2118,6 @@
apr_pool_t *pool)
{
apr_file_t *format_file = NULL;
- apr_status_t apr_err;
const char *format_contents = apr_psprintf (pool, "%d\n", version);
/* We only promise to handle non-negative integers. */
@@ -2113,10 +2130,8 @@
APR_WRITE | APR_CREATE, APR_OS_DEFAULT,
pool));
/* ...dump out our version number string... */
- apr_err = apr_file_write_full (format_file, format_contents,
- strlen (format_contents), NULL);
- if (apr_err)
- return svn_error_wrap_apr (apr_err, "Can't write to '%s'", path);
+ SVN_ERR (svn_io_file_write_full (format_file, format_contents,
+ strlen (format_contents), NULL, pool));
/* ...and close the file. */
SVN_ERR (svn_io_file_close (format_file, pool));
@@ -2133,15 +2148,12 @@
apr_file_t *format_file;
char buf[80];
apr_size_t len;
- apr_status_t apr_err;
/* Read a chunk of data from PATH */
SVN_ERR (svn_io_file_open (&format_file, path, APR_READ,
APR_OS_DEFAULT, pool));
len = sizeof(buf);
- apr_err = apr_file_read (format_file, buf, &len);
- if (apr_err)
- return svn_error_wrap_apr (apr_err, "Can't read '%s'", path);
+ SVN_ERR (svn_io_file_read (format_file, buf, &len, pool));
/* If there was no data in PATH, return an error. */
if (len == 0)
@@ -2183,7 +2195,8 @@
const char *file2,
apr_pool_t *pool)
{
- apr_status_t status;
+ svn_error_t *err1;
+ svn_error_t *err2;
apr_size_t bytes_read1, bytes_read2;
char buf1[BUFSIZ], buf2[BUFSIZ];
apr_file_t *file1_h = NULL;
@@ -2198,15 +2211,17 @@
"contents_identical_p: open failed on file 2");
*identical_p = TRUE; /* assume TRUE, until disproved below */
- for (status = 0; ! APR_STATUS_IS_EOF(status); )
+ do
{
- status = apr_file_read_full (file1_h, buf1, sizeof(buf1),
&bytes_read1);
- if (status && !APR_STATUS_IS_EOF(status))
- return svn_error_wrap_apr (status, "Can't read '%s'", file1);
+ err1 = svn_io_file_read_full (file1_h, buf1,
+ sizeof(buf1), &bytes_read1, pool);
+ if (err1 && !APR_STATUS_IS_EOF(err1->apr_err))
+ return err1;
- status = apr_file_read_full (file2_h, buf2, sizeof(buf2),
&bytes_read2);
- if (status && !APR_STATUS_IS_EOF(status))
- return svn_error_wrap_apr (status, "Can't read '%s'", file2);
+ err2 = svn_io_file_read_full (file2_h, buf2,
+ sizeof(buf2), &bytes_read2, pool);
+ if (err2 && !APR_STATUS_IS_EOF(err2->apr_err))
+ return err2;
if ((bytes_read1 != bytes_read2)
|| (memcmp (buf1, buf2, bytes_read1)))
@@ -2214,8 +2229,11 @@
*identical_p = FALSE;
break;
}
- }
+ } while (! err1 && ! err2);
+ svn_error_clear (err1);
+ svn_error_clear (err2);
+
SVN_ERR (svn_io_file_close (file1_h, pool));
SVN_ERR (svn_io_file_close (file2_h, pool));
Index: subversion/libsvn_subr/stream.c
===================================================================
--- subversion/libsvn_subr/stream.c (revision 7953)
+++ subversion/libsvn_subr/stream.c (working copy)
@@ -223,13 +223,17 @@
read_handler_apr (void *baton, char *buffer, apr_size_t *len)
{
struct baton_apr *btn = baton;
- apr_status_t status;
+ svn_error_t *err;
- status = apr_file_read_full (btn->file, buffer, *len, len);
- if (status && ! APR_STATUS_IS_EOF(status))
- return svn_error_wrap_apr (status, NULL, "Can't read file");
- return SVN_NO_ERROR;
+ err = svn_io_file_read_full (btn->file, buffer, *len, len, btn->pool);
+ if (err && APR_STATUS_IS_EOF(err->apr_err))
+ {
+ svn_error_clear (err);
+ err = SVN_NO_ERROR;
+ }
+
+ return err;
}
--
+++ GMX - die erste Adresse für Mail, Message, More +++
Neu: Preissenkung für MMS und FreeMMS! http://www.gmx.net
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
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
Posted by Julian Foad <ju...@btopenworld.com>.
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