Re: svn commit: r18301 - trunk/subversion/libsvn_ra_serf

[Daniel Rall <dl...@collab.net>]

On Mon, 30 Jan 2006, jerenkrantz@tigris.org wrote:
...
> +static const char *
> +find_attr(const char **attrs, const char *attr_name)
> +{
> +  const char *attr_val = NULL;
> +  const char **tmp_attrs = attrs;
> +
> +  while (*tmp_attrs)
> +    {
> +      if (strcmp(*tmp_attrs, attr_name) == 0)
> +        {
> +          attr_val = attrs[1];
> +        }
> +      tmp_attrs += 2;
> +    }
> +
> +  return attr_val;
> +}

Is it correct to continue to overwrite ATTR_VAL (always with the first
value from ATTRS) until ATTR_NAME is no longer found in ATTRS (which I
assume is a list of name/value pairs), rather than stopping looking
after the first instance of ATTR_NAME is encountered, or at least
taking the new value for ATTR_NAME?  Answers would be useful in the
doc string.

A couple of uses of the function follow (to provide some context).

...
> @@ -1245,16 +1327,8 @@
>    if (!ctx->state && strcmp(prop_name.name, "open-directory") == 0)
>      {
>        const char *rev = NULL;
> -      const char **tmp_attrs = attrs;
>  
> -      while (*tmp_attrs)
> -        {
> -          if (strcmp(*tmp_attrs, "rev") == 0)
> -            {
> -              rev = attrs[1];
> -            }
> -          tmp_attrs += 2;
> -        }
> +      rev = find_attr(attrs, "rev");
>  
>        if (!rev)
>          {
...
> +  else if (ctx->state && 
> +           (ctx->state->state == OPEN_DIR || ctx->state->state == ADD_DIR) &&
> +           strcmp(prop_name.name, "add-directory") == 0)
> +    {
> +      const char *dir_name = NULL;
> +      report_dir_t *dir_info;
> +
> +      dir_name = find_attr(attrs, "name");
> +
> +      push_state(ctx, ADD_DIR);
...

Re: svn commit: r18301 - trunk/subversion/libsvn_ra_serf

[Justin Erenkrantz <ju...@erenkrantz.com>]

On 1/31/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> More than one attribute with the same name in a start or empty-element tag
> is not well-formed XML, so the parser has to error out in that case.

Right.  Hopefully expat should be catching that instead of relying
upon me to detect that.  =)

> OTOH, there's no reason to not break early, but that's a minor
> optimization.

Yes, it could do so.

All of the XML code is going to have to be tweaked to better support
namespaces in the near-term.  When I get around to that, I'll add in a
break.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r18301 - trunk/subversion/libsvn_ra_serf

["Peter N. Lundblad" <pe...@famlundblad.se>]

On Tue, 31 Jan 2006, Daniel Rall wrote:

> On Mon, 30 Jan 2006, jerenkrantz@tigris.org wrote:
> ...
> > +static const char *
> > +find_attr(const char **attrs, const char *attr_name)
> > +{
> > +  const char *attr_val = NULL;
> > +  const char **tmp_attrs = attrs;
> > +
> > +  while (*tmp_attrs)
> > +    {
> > +      if (strcmp(*tmp_attrs, attr_name) == 0)
> > +        {
> > +          attr_val = attrs[1];
> > +        }
> > +      tmp_attrs += 2;
> > +    }
> > +
> > +  return attr_val;
> > +}
>
> Is it correct to continue to overwrite ATTR_VAL (always with the first
> value from ATTRS) until ATTR_NAME is no longer found in ATTRS (which I
> assume is a list of name/value pairs), rather than stopping looking
> after the first instance of ATTR_NAME is encountered, or at least
> taking the new value for ATTR_NAME?  Answers would be useful in the
> doc string.
>
More than one attribute with the same name in a start or empty-element tag
is not well-formed XML, so the parser has to error out in that case.
OTOH, there's no reason to not break early, but that's a minor
optimization.

Thanks,
//Peter

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org