Woah... I just noticed what is going on here.
Why are we not using skel's or the svn protocol structures? I
seriously disagree with adding Yet Another Storage Format into our
code. This is ridiculous.
I'm not sure when this snuck into the code, but I'm a big -1 on another format.
Use skels or svn structures. But not a new format.
-g
On Thu, Nov 20, 2008 at 16:45, <julianfoad_at_tigris.org> wrote:
> Author: julianfoad
> Date: Thu Nov 20 16:45:12 2008
> New Revision: 34293
>
> Log:
> Refactor the tree conflict storage code some more.
>
> * subversion/libsvn_wc/tree_conflicts.c
> (advance_on_match): Delete.
> (read_desc_separator): New function.
> (enum_mapping_t): New type.
> (node_kind_map, operation_map, action_map, reason_map): New data.
> (read_enum_field): New function.
> (read_victim_path, read_node_kind, read_operation, read_action, read_reason):
> Delete.
> (read_one_tree_conflict): Simplify, using the new functions and data.
> (svn_wc__read_tree_conflicts): Tweak and simplify.
> (write_enum_field): New functions.
> (svn_wc__write_tree_conflicts): Simplify, using write_enum_field().
>
> Modified:
> branches/tc_url_rev/subversion/libsvn_wc/tree_conflicts.c
>
> Modified: branches/tc_url_rev/subversion/libsvn_wc/tree_conflicts.c
> URL: http://svn.collab.net/viewvc/svn/branches/tc_url_rev/subversion/libsvn_wc/tree_conflicts.c?pathrev=34293&r1=34292&r2=34293
> ==============================================================================
> --- branches/tc_url_rev/subversion/libsvn_wc/tree_conflicts.c Thu Nov 20 16:26:45 2008 (r34292)
> +++ branches/tc_url_rev/subversion/libsvn_wc/tree_conflicts.c Thu Nov 20 16:45:12 2008 (r34293)
> @@ -30,21 +30,6 @@ static const char field_separator = SVN_
> static const char desc_separator = SVN_WC__TREE_CONFLICT_DESC_SEPARATOR;
> static const char escape_char = SVN_WC__TREE_CONFLICT_ESCAPE_CHAR;
>
> -/* If **INPUT starts with *TOKEN, advance *INPUT by the length of *TOKEN
> - * and return TRUE. Else, return FALSE and leave *INPUT alone. */
> -static svn_boolean_t
> -advance_on_match(const char **input, const char *token)
> -{
> - int len = strlen(token);
> - if ((strncmp(*input, token, len)) == 0)
> - {
> - *input += len;
> - return TRUE;
> - }
> - else
> - return FALSE;
> -}
> -
> /* Ensure the next character at position *START is a field separator, and
> * advance *START past it. */
> static svn_error_t *
> @@ -53,7 +38,22 @@ read_field_separator(const char **start,
> {
> if (*start >= end || **start != field_separator)
> return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Missing field delimiter in tree conflict description"));
> + _("Missing field separator in tree conflict description"));
> +
> + (*start)++;
> + return SVN_NO_ERROR;
> +}
> +
> +/* Ensure the next character at position *START is a description separator,
> + * and advance *START past it. */
> +static svn_error_t *
> +read_desc_separator(const char **start,
> + const char *end)
> +{
> + if (*start >= end || **start != desc_separator)
> + return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> + _("No separator at end of tree conflict description, "
> + "even though there is still data left to read"));
>
> (*start)++;
> return SVN_NO_ERROR;
> @@ -61,7 +61,7 @@ read_field_separator(const char **start,
>
> /* Parse a string field out of the data pointed to by *START. Set *STR to a
> * copy of the unescaped string, allocated in POOL. The string may be empty.
> - * Stop reading at an unescaped field- or description delimiter, and never
> + * Stop reading at an unescaped field- or description separator, and never
> * read past END.
> * After reading, make *START point to the character after the field.
> */
> @@ -106,148 +106,89 @@ read_string_field(const char **str,
> return SVN_NO_ERROR;
> }
>
> -/* Parse the 'victim path' field pointed to by *START. Modify the 'path'
> - * field of *CONFLICT by appending the victim name to its existing value.
> - * Stop reading at a field delimiter and never read past END.
> - * After reading, make *START point to the character after the field.
> - * Do all allocations in POOL.
> - */
> -static svn_error_t *
> -read_victim_path(svn_wc_conflict_description_t *conflict,
> - const char **start,
> - const char *end,
> - apr_pool_t *pool)
> +/* A mapping between a string STR and an enumeration value VAL. */
> +typedef struct enum_mapping_t
> {
> - const char *victim_basename;
> -
> - SVN_ERR(read_string_field(&victim_basename, start, end, pool));
> -
> - if (victim_basename[0] == '\0')
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Empty 'victim' field in tree conflict "
> - "description"));
> -
> - conflict->path = svn_path_join(conflict->path, victim_basename, pool);
> -
> - return SVN_NO_ERROR;
> -}
> -
> -/* Parse the 'node_kind' field pointed to by *START into the
> - * the tree conflict descriptor pointed to by DESC.
> + const char *str;
> + int val;
> +} enum_mapping_t;
> +
> +/* A map for svn_node_kind_t values. */
> +static const enum_mapping_t node_kind_map[] =
> +{
> + { SVN_WC__NODE_NONE, svn_node_none },
> + { SVN_WC__NODE_FILE, svn_node_file },
> + { SVN_WC__NODE_DIR, svn_node_dir },
> + { "", svn_node_unknown },
> + { NULL, 0 }
> +};
> +
> +/* A map for svn_wc_operation_t values. */
> +static const enum_mapping_t operation_map[] =
> +{
> + { SVN_WC__OPERATION_UPDATE, svn_wc_operation_update },
> + { SVN_WC__OPERATION_SWITCH, svn_wc_operation_switch },
> + { SVN_WC__OPERATION_MERGE, svn_wc_operation_merge },
> + { NULL, 0 }
> +};
> +
> +/* A map for svn_wc_conflict_action_t values. */
> +static const enum_mapping_t action_map[] =
> +{
> + { SVN_WC__CONFLICT_ACTION_EDITED, svn_wc_conflict_action_edit },
> + { SVN_WC__CONFLICT_ACTION_DELETED, svn_wc_conflict_action_delete },
> + { SVN_WC__CONFLICT_ACTION_ADDED, svn_wc_conflict_action_add },
> + { NULL, 0 }
> +};
> +
> +/* A map for svn_wc_conflict_reason_t values. */
> +static const enum_mapping_t reason_map[] =
> +{
> + { SVN_WC__CONFLICT_REASON_EDITED, svn_wc_conflict_reason_edited },
> + { SVN_WC__CONFLICT_REASON_DELETED, svn_wc_conflict_reason_deleted },
> + { SVN_WC__CONFLICT_REASON_MISSING, svn_wc_conflict_reason_missing },
> + { SVN_WC__CONFLICT_REASON_OBSTRUCTED, svn_wc_conflict_reason_obstructed },
> + { SVN_WC__CONFLICT_REASON_ADDED, svn_wc_conflict_reason_added },
> + { NULL, 0 }
> +};
> +
> +/* Parse the enumeration field pointed to by *START into *RESULT as a plain
> + * 'int', using MAP to convert from strings to enumeration values.
> + * In MAP, a null STR field marks the end of the map.
> * Don't read further than END.
> * After reading, make *START point to the character after the field.
> */
> static svn_error_t *
> -read_node_kind(svn_wc_conflict_description_t *conflict,
> - const char **start,
> - const char *end)
> -{
> - if (*start >= end)
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Expected tree conflict data but got none"));
> -
> - if (advance_on_match(start, SVN_WC__NODE_FILE))
> - conflict->node_kind = svn_node_file;
> - else if (advance_on_match(start, SVN_WC__NODE_DIR))
> - conflict->node_kind = svn_node_dir;
> - else
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Invalid 'node_kind' field in tree conflict description"));
> -
> - return SVN_NO_ERROR;
> -}
> -
> -/* Parse the 'operation' field pointed to by *START into the
> - * the tree conflict descriptor pointed to by DESC.
> - * Don't read further than END.
> - * After reading, make *START point to the character after the field.
> - */
> -static svn_error_t *
> -read_operation(svn_wc_conflict_description_t *conflict,
> - const char **start,
> - const char *end)
> -{
> - if (*start >= end)
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Expected tree conflict data but got none"));
> -
> - if (advance_on_match(start, SVN_WC__OPERATION_UPDATE))
> - conflict->operation = svn_wc_operation_update;
> - else if (advance_on_match(start, SVN_WC__OPERATION_SWITCH))
> - conflict->operation = svn_wc_operation_switch;
> - else if (advance_on_match(start, SVN_WC__OPERATION_MERGE))
> - conflict->operation = svn_wc_operation_merge;
> - else
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Invalid 'operation' field in tree conflict description"));
> -
> - return SVN_NO_ERROR;
> -}
> -
> -/* Parse the 'action' field pointed to by *START into the
> - * the tree conflict descriptor pointed to by DESC.
> - * Don't read further than END.
> - * After reading, make *START point to the character after the field.
> - */
> -static svn_error_t *
> -read_action(svn_wc_conflict_description_t *conflict,
> - const char **start,
> - const char *end)
> -{
> - if (*start >= end)
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Expected tree conflict data but got none"));
> -
> - if (advance_on_match(start, SVN_WC__CONFLICT_ACTION_EDITED))
> - conflict->action = svn_wc_conflict_action_edit;
> - else if (advance_on_match(start, SVN_WC__CONFLICT_ACTION_DELETED))
> - conflict->action = svn_wc_conflict_action_delete;
> - else if (advance_on_match(start, SVN_WC__CONFLICT_ACTION_ADDED))
> - conflict->action = svn_wc_conflict_action_add;
> - else
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Invalid 'action' field in tree conflict description"));
> +read_enum_field(int *result,
> + const enum_mapping_t *map,
> + const char **start,
> + const char *end,
> + apr_pool_t *pool)
> +{
> + const char *str;
> + int i;
>
> - return SVN_NO_ERROR;
> -}
> + SVN_ERR(read_string_field(&str, start, end, pool));
>
> -/* Parse the 'reason' field pointed to by *START into the
> - * the tree conflict descriptor pointed to by DESC.
> - * Don't read further than END.
> - * After reading, make *START point to the character after the field.
> - */
> -static svn_error_t *
> -read_reason(svn_wc_conflict_description_t *conflict,
> - const char **start,
> - const char *end)
> -{
> - if (*start >= end)
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Expected tree conflict data but got none"));
> -
> - if (advance_on_match(start, SVN_WC__CONFLICT_REASON_EDITED))
> - conflict->reason = svn_wc_conflict_reason_edited;
> - else if (advance_on_match(start, SVN_WC__CONFLICT_REASON_DELETED))
> - conflict->reason = svn_wc_conflict_reason_deleted;
> - else if (advance_on_match(start, SVN_WC__CONFLICT_REASON_MISSING))
> - conflict->reason = svn_wc_conflict_reason_missing;
> - else if (advance_on_match(start, SVN_WC__CONFLICT_REASON_OBSTRUCTED))
> - conflict->reason = svn_wc_conflict_reason_obstructed;
> - else if (advance_on_match(start, SVN_WC__CONFLICT_REASON_ADDED))
> - conflict->reason = svn_wc_conflict_reason_added;
> - else
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Invalid 'reason' field in tree conflict description"));
> + /* Find STR in MAP; error if not found. */
> + for (i = 0; ; i++)
> + {
> + if (map[i].str == NULL)
> + return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> + _("Unknown enumeration value in tree conflict "
> + "description"));
> + if (strcmp(str, map[i].str) == 0)
> + break;
> + }
>
> + *result = map[i].val;
> return SVN_NO_ERROR;
> }
>
> /* Parse a newly allocated svn_wc_conflict_description_t object from the
> * character string pointed to by *START. Return the result in *CONFLICT.
> - * Don't read further than END. If a description separator is found in
> - * the string after the conflict description that was read, set *START
> - * to the first character of the next conflict description.
> - * Otherwise, set *START to NULL.
> + * Don't read further than END. Set *START to point to the next character
> + * after the description that was read.
> * DIR_PATH is the path to the WC directory whose conflicts are being read.
> * Do all allocations in pool.
> */
> @@ -258,37 +199,49 @@ read_one_tree_conflict(svn_wc_conflict_d
> const char *dir_path,
> apr_pool_t *pool)
> {
> - if (*start >= end)
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Expected tree conflict data but got none"));
> + const char *victim_basename;
> + svn_node_kind_t node_kind;
> + svn_wc_operation_t operation;
> + int n;
>
> - *conflict = svn_wc_conflict_description_create_tree(
> - dir_path, NULL, svn_node_none, 0, pool);
> + SVN_ERR_ASSERT(*start < end);
>
> - /* Each of these modifies *START ! */
> - SVN_ERR(read_victim_path(*conflict, start, end, pool));
> + /* Each read_...() call modifies *START ! */
> +
> + /* victim basename */
> + SVN_ERR(read_string_field(&victim_basename, start, end, pool));
> + if (victim_basename[0] == '\0')
> + return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> + _("Empty 'victim' field in tree conflict "
> + "description"));
> SVN_ERR(read_field_separator(start, end));
> - SVN_ERR(read_node_kind(*conflict, start, end));
> +
> + /* node_kind */
> + SVN_ERR(read_enum_field(&n, node_kind_map, start, end, pool));
> + node_kind = (svn_node_kind_t)n;
> + if (node_kind != svn_node_file && node_kind != svn_node_dir)
> + return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> + _("Invalid 'node_kind' field in tree conflict description"));
> SVN_ERR(read_field_separator(start, end));
> - SVN_ERR(read_operation(*conflict, start, end));
> +
> + /* operation */
> + SVN_ERR(read_enum_field(&n, operation_map, start, end, pool));
> + operation = (svn_wc_operation_t)n;
> SVN_ERR(read_field_separator(start, end));
> - SVN_ERR(read_action(*conflict, start, end));
> +
> + /* Construct the description object */
> + *conflict = svn_wc_conflict_description_create_tree(
> + svn_path_join(dir_path, victim_basename, pool),
> + NULL, node_kind, operation, pool);
> +
> + /* action */
> + SVN_ERR(read_enum_field(&n, action_map, start, end, pool));
> + (*conflict)->action = (svn_wc_conflict_action_t)n;
> SVN_ERR(read_field_separator(start, end));
> - SVN_ERR(read_reason(*conflict, start, end));
>
> - /* *START should now point to a description separator
> - * if there are any descriptions left. */
> - if (**start == desc_separator)
> - (*start)++;
> - else
> - {
> - if (*start >= end)
> - *start = NULL;
> - else
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("No delimiter at end of tree conflict description, "
> - "even though there is still data left to read"));
> - }
> + /* reason */
> + SVN_ERR(read_enum_field(&n, reason_map, start, end, pool));
> + (*conflict)->reason = (svn_wc_conflict_reason_t)n;
>
> return SVN_NO_ERROR;
> }
> @@ -300,33 +253,28 @@ svn_wc__read_tree_conflicts(apr_array_he
> apr_pool_t *pool)
> {
> const char *start, *end;
> - svn_wc_conflict_description_t *conflict = NULL;
>
> if (conflict_data == NULL)
> - {
> - return SVN_NO_ERROR;
> - }
> + return SVN_NO_ERROR;
>
> SVN_ERR_ASSERT(*conflicts);
>
> start = conflict_data;
> end = start + strlen(start);
>
> - while (start != NULL && start <= end) /* Yes, '<=', because 'start == end'
> - is a special case that is dealt
> - with further down the call chain. */
> + while (start < end)
> {
> + svn_wc_conflict_description_t *conflict;
> +
> SVN_ERR(read_one_tree_conflict(&conflict, &start, end, dir_path, pool));
> if (conflict != NULL)
> APR_ARRAY_PUSH(*conflicts, svn_wc_conflict_description_t *) = conflict;
> - }
>
> - if (start != NULL)
> - /* Not all conflicts have been read from the entry, but no error
> - * has been thrown yet. We should not even be here! */
> - return svn_error_create(SVN_ERR_WC_CORRUPT, NULL,
> - _("Invalid tree conflict data in 'entries' file, "
> - "but no idea what went wrong"));
> + /* *START should now point to a description separator
> + * if there are any descriptions left. */
> + if (start < end)
> + SVN_ERR(read_desc_separator(&start, end));
> + }
>
> return SVN_NO_ERROR;
> }
> @@ -352,6 +300,25 @@ write_string_field(svn_stringbuf_t *buf,
> }
> }
>
> +/* Append to BUF the string corresponding to enumeration value N, as found
> + * in MAP. */
> +static svn_error_t *
> +write_enum_field(svn_stringbuf_t *buf,
> + const enum_mapping_t *map,
> + int n)
> +{
> + int i;
> +
> + for (i = 0; ; i++)
> + {
> + SVN_ERR_ASSERT(map[i].str != NULL);
> + if (map[i].val == n)
> + break;
> + }
> + svn_stringbuf_appendcstr(buf, map[i].str);
> + return SVN_NO_ERROR;
> +}
> +
> /*
> * This function could be static, but we need to link to it
> * in a unit test in tests/libsvn_wc/, so it isn't.
> @@ -370,81 +337,32 @@ svn_wc__write_tree_conflicts(char **conf
> const svn_wc_conflict_description_t *conflict =
> APR_ARRAY_IDX(conflicts, i, svn_wc_conflict_description_t *);
>
> - /* Escape separator chars while writing victim path. */
> + /* Victim path (escaping separator chars). */
> path = svn_path_basename(conflict->path, pool);
> SVN_ERR_ASSERT(strlen(path) > 0);
> write_string_field(buf, path);
>
> svn_stringbuf_appendbytes(buf, &field_separator, 1);
>
> - switch (conflict->node_kind)
> - {
> - case svn_node_dir:
> - svn_stringbuf_appendcstr(buf, SVN_WC__NODE_DIR);
> - break;
> - case svn_node_file:
> - svn_stringbuf_appendcstr(buf, SVN_WC__NODE_FILE);
> - break;
> - default:
> - SVN_ERR_MALFUNCTION();
> - }
> + /* node_kind */
> + SVN_ERR_ASSERT(conflict->node_kind == svn_node_dir
> + || conflict->node_kind == svn_node_file);
> + SVN_ERR(write_enum_field(buf, node_kind_map, conflict->node_kind));
>
> svn_stringbuf_appendbytes(buf, &field_separator, 1);
>
> - switch (conflict->operation)
> - {
> - case svn_wc_operation_update:
> - svn_stringbuf_appendcstr(buf, SVN_WC__OPERATION_UPDATE);
> - break;
> - case svn_wc_operation_switch:
> - svn_stringbuf_appendcstr(buf, SVN_WC__OPERATION_SWITCH);
> - break;
> - case svn_wc_operation_merge:
> - svn_stringbuf_appendcstr(buf, SVN_WC__OPERATION_MERGE);
> - break;
> - default:
> - SVN_ERR_MALFUNCTION();
> - }
> + /* operation */
> + SVN_ERR(write_enum_field(buf, operation_map, conflict->operation));
>
> svn_stringbuf_appendbytes(buf, &field_separator, 1);
>
> - switch (conflict->action)
> - {
> - case svn_wc_conflict_action_edit:
> - svn_stringbuf_appendcstr(buf, SVN_WC__CONFLICT_ACTION_EDITED);
> - break;
> - case svn_wc_conflict_action_delete:
> - svn_stringbuf_appendcstr(buf, SVN_WC__CONFLICT_ACTION_DELETED);
> - break;
> - case svn_wc_conflict_action_add:
> - svn_stringbuf_appendcstr(buf, SVN_WC__CONFLICT_ACTION_ADDED);
> - break;
> - default:
> - SVN_ERR_MALFUNCTION();
> - }
> + /* action */
> + SVN_ERR(write_enum_field(buf, action_map, conflict->action));
>
> svn_stringbuf_appendbytes(buf, &field_separator, 1);
>
> - switch (conflict->reason)
> - {
> - case svn_wc_conflict_reason_edited:
> - svn_stringbuf_appendcstr(buf, SVN_WC__CONFLICT_REASON_EDITED);
> - break;
> - case svn_wc_conflict_reason_deleted:
> - svn_stringbuf_appendcstr(buf, SVN_WC__CONFLICT_REASON_DELETED);
> - break;
> - case svn_wc_conflict_reason_added:
> - svn_stringbuf_appendcstr(buf, SVN_WC__CONFLICT_REASON_ADDED);
> - break;
> - case svn_wc_conflict_reason_missing:
> - svn_stringbuf_appendcstr(buf, SVN_WC__CONFLICT_REASON_MISSING);
> - break;
> - case svn_wc_conflict_reason_obstructed:
> - svn_stringbuf_appendcstr(buf, SVN_WC__CONFLICT_REASON_OBSTRUCTED);
> - break;
> - default:
> - SVN_ERR_MALFUNCTION();
> - }
> + /* reason */
> + SVN_ERR(write_enum_field(buf, reason_map, conflict->reason));
>
> if (i < (conflicts->nelts - 1))
> svn_stringbuf_appendbytes(buf, &desc_separator, 1);
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: svn-unsubscribe_at_subversion.tigris.org
> For additional commands, e-mail: svn-help_at_subversion.tigris.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-11-21 05:00:52 CET