[PATCH v3 1/3] msxml3: Implement ::get_attributes() for "xml" processing instruction node. (Resend)

[Dmitry Timoshkov]

Nikolay Sivov <nsivov at codeweavers.com> wrote:

> > +static HRESULT xml_get_value(xmlChar **p, xmlChar **value)
> > +{
> > +    xmlChar *v;
> > +    int len;
> > +
> > +    while (isspace(**p)) *p += 1;
> > +    if (**p != '=') return XML_E_MISSINGEQUALS;
> > +    *p += 1;
> > +
> > +    while (isspace(**p)) *p += 1;
> > +    if (**p != '"') return XML_E_MISSINGQUOTE;
> > +    *p += 1;
> > +
> > +    v = *p;
> > +    while (**p && **p != '"') *p += 1;
> > +    if (!**p) return XML_E_EXPECTINGCLOSEQUOTE;
> > +    len = *p - v;
> > +    if (!len) return XML_E_MISSINGNAME;
> > +    *p += 1;
> > +
> > +    *value = heap_alloc(len + 1);
> > +    if (!*value) return E_OUTOFMEMORY;
> > +    memcpy(*value, v, len);
> > +    *(*value + len) = 0;
> > +
> > +    return S_OK;
> > +}
> Is it useful to be that specific about error codes (and including
> xmlparser.h for that)? Since that pi node is a product of already
> performed parsing, it seems that it should always be valid anyway. I
> don't see them reaching out of dom_pi_get_attributes():

You are probably correct that using such specific error codes is not
very useful, however I wanted to avoid generic E_FAIL, and particular
error values better help in diagnosing parser bugs.

> > -        FIXME("created dummy map for <?xml ?>\n");
> > +        if (!This->node.node->properties)
> > +        {
> > +            hr = parse_xml_decl(This->node.node);
> > +            if (hr != S_OK)
> > +            {
> > +                SysFreeString(name);
> > +                return S_FALSE;
> > +            }
> > +        }
> > +
> So it's either S_OK or S_FALSE.

Right. Thanks for the helpful review.

-- 
Dmitry.