Re: [xml] Internal and External Validating Bug

[Daniel Veillard <veillard redhat com>]

On Tue, Mar 20, 2001 at 03:17:59AM +0000, Gary Pennington wrote:
> Hi,
> 
> This took some finding....

  Hi Gary, thanks for chasing this one,

> So, it turns out that it's to do with passing around uninitialized pointers
> and the behaviour you get is compiler (and hence) platform dependant.
> 
> I could only get the core dump on Solaris x86 @(not SPARC) built with the
> Forte compiler. If I built with gcc, then I didn't get the error (I'm
> guessing this is because a flag in the gcc compiler is set to cause all
> uninitialized pointers to be set to zero - any more informed gcc users care
> to comment?)

  Hum, no of course ! Gcc doesn't do this kind of workaround

> Anyway, I managed to generate the error  and the problem is in parser.c,
> xmlParseNotationDecl.
> 
> The Pubid variable is not initialized explicitly and the code path taken in
> your example means that it is never initialized. In other functions the
> value of Pubid is compared to NULL and since it isn't NULL invalid
> decisions are made and the code dumps core in an unpredictable fashion.
> 
> Patch to parser.c to fix this particular problem is attached.

  Well actually the bug is in xmlParseExternalID() which doesn't do
the initialization of the area it get passed for publicID, resulting in
the same problem, Pubid in xmlParseNotationDecl() is not initialized.
It's not a reason to systematically set all pointers to a priori.

> I notice that there may be many uninitialized pointers lurking in this
> fashion to catch the unwary. Would it be worth explicitly setting all
> pointers to NULL to prevent this kind of problem for non-gcc compiler
> users?

  Hum, when I see

  xmlChar *name;

  name = xmlParseNameComplex(ctxt);

I really don't see why we should add the 

  xmlChar *name = NULL;

The *real* fix is to find the place in the different execution paths
where such a variable is not initialized, and honnestly I think there
isn't many. One was found in xmlParseNotationDecl for the very simple
reason taht *nobody* (except maybe DocBook) ever use the NOTATION
construct in real apps :-\

So no the right approach IMHO is to make regression tests and find
possibles problems, not put a penaly of double initialization of all
pointers on the stack because it is easier to do.

> I've also attached a patch to configure which is still broken on Solaris
> when not using gcc and has been since 2.3.3

Right problem was reported and fixed independantly,

  thanks for the report and the analysis, could you check that
the enclosed patch fixes the problem on your plateform ?

Daniel

-- 
Daniel Veillard      | Red Hat Network http://redhat.com/products/network/
veillard redhat com  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/

diff -c -r1.125 parser.c
*** parser.c	2001/03/21 18:06:15	1.125
--- parser.c	2001/03/22 13:12:52
***************
*** 2588,2593 ****
--- 2588,2595 ----
      xmlChar *URI = NULL;
  
      SHRINK;
+ 
+     *publicID = NULL;
      if ((RAW == 'S') && (NXT(1) == 'Y') &&
           (NXT(2) == 'S') && (NXT(3) == 'T') &&
  	 (NXT(4) == 'E') && (NXT(5) == 'M')) {