You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Peter N. Lundblad" <pe...@famlundblad.se> on 2006/01/26 15:24:50 UTC
Re: svn commit: r18245 - trunk/subversion/libsvn_ra_serf
On Thu, 26 Jan 2006 jerenkrantz@tigris.org wrote:
> Modified: trunk/subversion/libsvn_ra_serf/serf.c
> Url: http://svn.collab.net/viewcvs/svn/trunk/subversion/libsvn_ra_serf/serf.c?rev=18245&p1=trunk/subversion/libsvn_ra_serf/serf.c&p2=trunk/subversion/libsvn_ra_serf/serf.c&r1=18244&r2=18245
> ==============================================================================
> --- trunk/subversion/libsvn_ra_serf/serf.c (original)
> +++ trunk/subversion/libsvn_ra_serf/serf.c Thu Jan 26 01:45:11 2006
> @@ -20,6 +20,8 @@
>
> #include <apr_uri.h>
>
> +#include <expat.h>
> +
> #include <serf.h>
>
> #include "svn_pools.h"
> @@ -168,6 +170,12 @@
> const char *name;
> } dav_props_t;
>
> +typedef struct ns {
We usually use the same name for the struct tag and the typedef.
> + const char *namespace;
> + const char *val;
There is no documentation, but I guess the namespace field is the
namespace identifier (i.e. DAV in DAV:blah) and the val field is the
namespace URI. Is that correct?
> + struct ns *next;
> +} ns_t;
> +
> static const char *
> fetch_prop (apr_hash_t *props,
> const char *path,
> @@ -265,9 +273,127 @@
> const dav_props_t *find_props;
> apr_hash_t *ret_props;
>
> + XML_Parser xmlp;
> + ns_t *ns_list;
We have a wrapper for expat in svn_xml.h that gives us some error
handling. Maybe use that (and extend it with namespace support)?
> +
> +static void XMLCALL
> +start_propfind(void *userData, const char *name, const char **attrs)
> +{
> + propfind_context_t *ctx = userData;
> + const char *ns;
> + char *colon;
> +
> + /* check for new namespaces */
> + while (*attrs)
> + {
> + if (strncmp(*attrs, "xmlns", 5) == 0)
> + {
> + const char *attr, *attr_val;
> + attr = attrs[0] + 6;
Woops, this will point at garbage if the attribute name is just xmlns.
> + attr_val = attrs[1];
> + define_ns(ctx, attr, attr_val);
OK, so val is the URI. Might choose a better name.
> + }
> + attrs += 2;
> + }
> +
> + colon = strchr(name, ':');
> + if (colon)
> + {
> + /* look up name space */
> + *colon = '\0';
> + ns = expand_ns(ctx, name);
> + if (!ns)
> + {
> + abort();
> + }
> + name = colon + 1;
> + }
> + else
> + {
> + /* default namespace */
> + ns = "";
Do DAV prohibit declaring another default namespace or something?
> + }
> +
> + if (ctx->in_prop && !ctx->name)
> + {
> + ctx->ns = ns;
> + ctx->name = apr_pstrdup(ctx->pool, name);
> + /* we want to flag the cdata handler to pick up what's next. */
> + ctx->collect_cdata = 1;
s/1/TRUE/
> + }
> +
> + /* check for 'prop' */
> + if (!ctx->in_prop && strcasecmp(name, "prop") == 0)
No my DAV ingorance shines through:-(, but are names case-insensitive in
DAV?
> + {
> + ctx->in_prop = 1;
Same about the bool. Here and further down, so I don't note them all.
> + }
> +}
> +
> +static void XMLCALL
> +end_propfind(void *userData, const char *name)
> +{
> + propfind_context_t *ctx = userData;
> + if (ctx->collect_cdata)
> + {
> + if (ctx->val)
> + {
> + set_prop(ctx->ret_props, ctx->path, ctx->ns, ctx->name, ctx->val,
> + ctx->pool);
> + }
> + ctx->collect_cdata = 0;
> + ctx->name = NULL;
> + ctx->val = NULL;
> + }
> +}
> +
To be correct, you need to pop the namespaces of this element, but you
know that. (I also saw the comment about the state of this code in the
log message about check-pointing).
> +static void XMLCALL
> +cdata_propfind(void *userData, const char *data, int len)
> +{
> + propfind_context_t *ctx = userData;
> + if (ctx->collect_cdata)
> + {
> + ctx->val = apr_pstrndup(ctx->pool, data, len);
> + }
> +
expat doesn't guarantee that CDATA in an element come in one chunk, so you
need to collect the CDATA from consecutive calls.
> +}
> +
Best,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18245 - trunk/subversion/libsvn_ra_serf
Posted by "Peter N. Lundblad" <pe...@famlundblad.se>.
On Thu, 26 Jan 2006, Justin Erenkrantz wrote:
> On 1/26/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> > > + const char *attr, *attr_val;
> > > + attr = attrs[0] + 6;
> >
> > Woops, this will point at garbage if the attribute name is just xmlns.
>
> I'm not yet trying to deal with bogus XML. I just want to get enough
> parsing functional so that I can make progress on the other stuff.
>
OK, maybe a todo comment oculd say so so it doesn't get missed. Also
pointing out that this isn't bogus XML at all.
> > > + /* default namespace */
> > > + ns = "";
> >
> > Do DAV prohibit declaring another default namespace or something?
>
> It needs some namespace... *shrug*
>
Huh? What do you mean? declaring a default namespace with "xmlns='url'"
means that unprefixed element names will belong to that namespace. Are
you hard-coding assumptions about namespace prefixes (or rather no
prefixes) that the server will use?
Thanks,
//Peter
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: svn commit: r18245 - trunk/subversion/libsvn_ra_serf
Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On 1/26/06, Peter N. Lundblad <pe...@famlundblad.se> wrote:
> We usually use the same name for the struct tag and the typedef.
Fixed.
> There is no documentation, but I guess the namespace field is the
> namespace identifier (i.e. DAV in DAV:blah) and the val field is the
> namespace URI. Is that correct?
Yes - I updated the fields to be more precise.
> > + XML_Parser xmlp;
> > + ns_t *ns_list;
>
> We have a wrapper for expat in svn_xml.h that gives us some error
> handling. Maybe use that (and extend it with namespace support)?
Perhaps. But, my expectation is a lot of the XML parsing code is
going to go into serf rather than stay in Subversion. I'm not sure
yet.
> > + const char *attr, *attr_val;
> > + attr = attrs[0] + 6;
>
> Woops, this will point at garbage if the attribute name is just xmlns.
I'm not yet trying to deal with bogus XML. I just want to get enough
parsing functional so that I can make progress on the other stuff.
> > + /* default namespace */
> > + ns = "";
>
> Do DAV prohibit declaring another default namespace or something?
It needs some namespace... *shrug*
> s/1/TRUE/
Fixed.
> > + }
> > +
> > + /* check for 'prop' */
> > + if (!ctx->in_prop && strcasecmp(name, "prop") == 0)
>
> No my DAV ingorance shines through:-(, but are names case-insensitive in
> DAV?
Greg says no in another post. ;-)
> To be correct, you need to pop the namespaces of this element, but you
> know that. (I also saw the comment about the state of this code in the
> log message about check-pointing).
Yah. I added a FIXME to note this.
> expat doesn't guarantee that CDATA in an element come in one chunk, so you
> need to collect the CDATA from consecutive calls.
I'll figure that out once I get a better idea where the XML parsing
code should live.
Thanks. -- justin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org