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

[Dmitry Timoshkov]

Dmitry Timoshkov <dmitry at baikal.ru> wrote:

> 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.

Also, according to the tests more verbose parse errors could be returned
by ::createProcessingInstruction(), and if desirable the parsing step
might be executed there. However, moving the parsing step requires large
restructuring of the node creation logic, and I'd prefer to not depend
these patches on it.

-- 
Dmitry.