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

Re: [PATCH] Fix 2441

From: Kamesh Jayachandran <kamesh_at_collab.net>
Date: 2006-01-10 10:14:10 CET

Thanks Julian, DLR, Garrett.
Julian,
Not sure how to reduce it further eventhough I could understand your
points on stuffs like some spurious newline introduced by the program
that generates the dump file.

Find the new patch attached.

With regards
Kamesh Jayachandran

[[[
Fix issue #2441, 'svnadmin load' malformed dumpfile errors are
intolerably vague

* subversion/libsvn_repos/load.c
  (contextual_header_block_error): New function to return a detailed
error message.
  (read_header_block): Call contextual_header_block_error().

]]]

Daniel Rall wrote:
> On Fri, 06 Jan 2006, Kamesh Jayachandran wrote:
>
>
>> Hi All,
>> Slightly better fix for #2441.
>>
>
> Thanks Kamesh! Last couple-few minor things.
>
>
>> [[[
>> Fix issue #2441, 'svnadmin load' malformed dumpfile errors are
>> intolerably vague
>>
>> * subversion/libsvn_repos/load.c
>> (contextual_error): will return a svn_error_t * indicating which
>> header block
>> problematic header occured with a header name and a reason for failure.
>>
>
> Usage of contextual_error() is missing from the change log message.
>
>
>> ]]]
>>
>>
>
>
>> Index: subversion/libsvn_repos/load.c
>> ===================================================================
>> --- subversion/libsvn_repos/load.c (revision 17993)
>> +++ subversion/libsvn_repos/load.c (working copy)
>> @@ -105,6 +105,56 @@
>> }
>>
>>
>> +/** This function gives a contextual clue of dump file malformed content. **/
>> +static svn_error_t *
>> +contextual_error (apr_hash_t *headers, char *key, char *reason)
>>
>
> Something like contextual_dumpfile_error() or
> contextual_header_block_error() would provide a more explicit name.
>
>
>> +{
>> + char *block=NULL;
>> + if ((block = (char *)apr_hash_get (headers,
>>
> ^
> We usually put a space here (and below):
>
> if ((block = (char *) apr_hash_get (headers,
>
>
>> + SVN_REPOS_DUMPFILE_REVISION_NUMBER,
>> + APR_HASH_KEY_STRING)) != NULL)
>>
> ^
> Now they'll line up. Bonus.
>
>
>> + {
>> + return svn_error_createf (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
>> + _("Found malformed revision %s header block "
>> + "near the line starting with a key %s %s "
>> + "in dumpfile stream"), block, key, reason);
>>
>
> Quote the key value '%s' to help the reader here and below.
>
>
>> + }
>> + else if ((block = (char *)apr_hash_get (headers,
>> + SVN_REPOS_DUMPFILE_NODE_PATH,
>> + APR_HASH_KEY_STRING)) != NULL)
>> + {
>> + return svn_error_createf (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
>> + _("Found malformed node %s header block "
>> + "near the line starting with a key %s %s "
>> + "in dumpfile stream"), block, key, reason);
>> + }
>> + else if ((block = (char *)apr_hash_get (headers,
>> + SVN_REPOS_DUMPFILE_UUID,
>> + APR_HASH_KEY_STRING)) != NULL)
>> + {
>> + return svn_error_createf (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
>> + _("Found malformed UUID header block "
>> + "near the line starting with a key %s %s "
>> + "in dumpfile stream"), key, reason);
>> + }
>> + else if ((block = (char *)apr_hash_get (headers,
>> + SVN_REPOS_DUMPFILE_MAGIC_HEADER,
>> + APR_HASH_KEY_STRING)) != NULL)
>> + {
>> + return svn_error_createf (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
>> + _("Found malformed magic header block "
>> + "near the line starting with a key %s %s "
>> + "in dumpfile stream"), key, reason);
>> + }
>> + else
>> + {
>> + return svn_error_createf (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
>> + _("Found malformed unknown header block "
>> + "near the line starting with a key %s %s "
>> + "in dumpfile stream"), key, reason);
>> + }
>> +}
>> +
>> /*----------------------------------------------------------------------*/
>>
>> /** The parser and related helper funcs **/
>> @@ -153,9 +203,8 @@
>> while (header_str->data[i] != ':')
>> {
>> if (header_str->data[i] == '\0')
>> - return svn_error_create (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
>> - _("Found malformed header block "
>> - "in dumpfile stream"));
>> + return contextual_error (*headers, header_str->data,
>> + "with no : character");
>>
>
> Quote the ':' character here and below.
>
>
>> i++;
>> }
>> /* Create a 'name' string and point to it. */
>> @@ -165,9 +214,8 @@
>> /* Skip over the NULL byte and the space following it. */
>> i += 2;
>> if (i > header_str->len)
>> - return svn_error_create (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
>> - _("Found malformed header block "
>> - "in dumpfile stream"));
>> + return contextual_error (*headers, header_str->data,
>> + "with no value after a : character");
>>
> ...
>

Index: subversion/libsvn_repos/load.c
===================================================================
--- subversion/libsvn_repos/load.c (revision 17993)
+++ subversion/libsvn_repos/load.c (working copy)
@@ -105,6 +105,58 @@
 }
 
 
+/* Compose and return an error describing the problem REASON with the
+ * dump file header KEY, and the block where these header key is located.
+ */
+static svn_error_t *
+contextual_header_block_error (apr_hash_t *headers, const char *key, const char *reason)
+{
+ char *block;
+ if ((block = apr_hash_get (headers,
+ SVN_REPOS_DUMPFILE_REVISION_NUMBER,
+ APR_HASH_KEY_STRING)) != NULL)
+ {
+ return svn_error_createf (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
+ _("Found malformed revision '%s' header block "
+ "near the line starting with a key '%s' %s "
+ "in dumpfile stream"), block, key, reason);
+ }
+ else if ((block = apr_hash_get (headers,
+ SVN_REPOS_DUMPFILE_NODE_PATH,
+ APR_HASH_KEY_STRING)) != NULL)
+ {
+ return svn_error_createf (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
+ _("Found malformed node '%s' header block "
+ "near the line starting with a key '%s' %s "
+ "in dumpfile stream"), block, key, reason);
+ }
+ else if ((block = apr_hash_get (headers,
+ SVN_REPOS_DUMPFILE_UUID,
+ APR_HASH_KEY_STRING)) != NULL)
+ {
+ return svn_error_createf (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
+ _("Found malformed UUID header block "
+ "near the line starting with a key '%s' %s "
+ "in dumpfile stream"), key, reason);
+ }
+ else if ((block = apr_hash_get (headers,
+ SVN_REPOS_DUMPFILE_MAGIC_HEADER,
+ APR_HASH_KEY_STRING)) != NULL)
+ {
+ return svn_error_createf (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
+ _("Found malformed magic header block "
+ "near the line starting with a key '%s' %s "
+ "in dumpfile stream"), key, reason);
+ }
+ else
+ {
+ return svn_error_createf (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
+ _("Found malformed unknown header block "
+ "near the line starting with a key '%s' %s "
+ "in dumpfile stream"), key, reason);
+ }
+}
+
 /*----------------------------------------------------------------------*/
 
 /** The parser and related helper funcs **/
@@ -153,9 +205,8 @@
       while (header_str->data[i] != ':')
         {
           if (header_str->data[i] == '\0')
- return svn_error_create (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
- _("Found malformed header block "
- "in dumpfile stream"));
+ return contextual_header_block_error (*headers, header_str->data,
+ _("with no ':' character"));
           i++;
         }
       /* Create a 'name' string and point to it. */
@@ -165,9 +216,8 @@
       /* Skip over the NULL byte and the space following it. */
       i += 2;
       if (i > header_str->len)
- return svn_error_create (SVN_ERR_STREAM_MALFORMED_DATA, NULL,
- _("Found malformed header block "
- "in dumpfile stream"));
+ return contextual_header_block_error (*headers, header_str->data,
+ _("with no value after a ':' character"));
 
       /* Point to the 'value' string. */
       value = header_str->data + i;

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jan 10 10:16:12 2006

This is an archived mail posted to the Subversion Dev mailing list.