[Expat-bugs] [ expat-Bugs-620343 ] segfault: badAPI/callbackinteraction

[Fred L. Drake, Jr.]

Karl Waclawek writes:
 > 1) defaultHandler call - related to your patch
 >    Current code:
 > 
 >         if (startElementHandler) {
 >           result = storeAtts(parser, enc, s, &(tag->name), &(tag->bindings));
 >           if (result)
 >             return result;
 >           if (startElementHandler)
 >             startElementHandler(handlerArg, tag->name.str,
 >                                 (const XML_Char **)atts);
 >         }
 >         else if (defaultHandler)
 >           reportDefault(parser, enc, s, next);
 > 
 > this should be changed to:
 > 
 >         if (startElementHandler) {
 >           result = storeAtts(parser, enc, s, &(tag->name), &(tag->bindings));
 >           if (result)
 >             return result;
 >           if (startElementHandler)
 >             startElementHandler(handlerArg, tag->name.str,
 >                                 (const XML_Char **)atts);
 >           else if (defaultHandler)
 >             reportDefault(parser, enc, s, next);
 >         }
 >         else if (defaultHandler)
 >           reportDefault(parser, enc, s, next);
 > 
 > since - as you noticed - the startElementHandler could be turned off
 > in the startNamespaceDeclHandler callback.

Agreed.  Please commit the change when you get a chance.

 > 2) Same as 1) for the empty element tag, but not as simple
 >    to solve. Consider these 4 cases:
 >    a) no element handler set
 >    b) startElementHandler set
 >    c) endElementHandler set
 >    d) both element handlers set
 > Now, when should the default handler be called?
 > The answers is difficult for case c), since the endElementHandler
 > does not report attributes. So, if we don't call the default handler
 > for c), attributes go completely unreported, but if we do, we will
 > double report the element name.
 > 
 > What is your opinion? I tend to think that the default handler
 > is the substitue for the startElementHandler rather than the
 > endElementHandler, so I would say, call the default handler for c).

The way I think of the default handler is that it is called for markup
that is not otherwise reported; the only tricky one is when there is
only an end element handler and default handler.  I'd err on the side
of not calling the default handler; the end element handler does
report the markup, but not necessarily in a comprehensive way.

Others who rely more on the default handler should chime in on this
topic.  ;-)

 > One could of course be fancy and check if there are any attributes.
 > If yes, call it, if no, don't. But nothing really satisfies.

No!  Too painful to explain and accomodate in the application.

 > The solution would be simple if start- and endElementHandler could
 > only be set or cleared together, but not independently.

By which I presume you mean that we also require both to be NULL or
both non-NULL.  That would be a real potential nuissance for current
users; even if an application uses both or none, it may use the
SetStart... and SetEnd... instead of Set(both) functions to make the
changes; this is certainly how the Python bindings work.  It's
possible to write the binding to call the Set(both) function, but more
painful to "pair up" settings of the handlers when changing them from
set to not-set without substantial API changes at the Python level
(dummy handlers would have to be introduced, and more state checked
and managed while changing the handlers).  I don't know if any of the
other bindings would suffer that limitation.

 > 3) It is possible that storeAtts gets called twice:
...
 > I suggest this fix:

This looks good; please check it in at your leisure!  ;-)


  -Fred

-- 
Fred L. Drake, Jr.  <fdrake at acm.org>
PythonLabs at Zope Corporation