[OPEN-ILS-DEV] Dubious code in mod_xmlbuilder.c

[Bill Erickson]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Scott McKellar wrote:
> In mod_xmlbuilder.c, xmlBuilderStartElement() contains the following
> fragment of code (condensed here for brevity):
> 
>     char* href = strdup(xmlSaxAttr( atts, "href" ));
>     if(href) {
> 
>         ...snip... 
>     }
> 
>     if(!node) {
>         apacheError("Unable to parse xinclude: %s", href );
>         free(href);
>         return;
>     }
>     free(href);
> 
> My first thought was: why are we checking the return code of 
> strdup() to see if it's NULL?  If strdup() ever returns NULL we 
> are almost certainly hosed, and cannot save ourselves simply by
> skipping over a short stretch of code.
> 
> In fact just after skipping that stretch we may find ourselves
> passing the NULL to apacheError(), which probably invokes undefined
> behavior.
> 
> Upon looking further I realized that xmlSaxAttr() may return NULL,
> which we then try to strdup, with unhappy results.  My system
> segfaults when I try to strdup a NULL.
> 
> My guess is that the test for a NULL from strdup() was intended
> conceptually to test for a NULL return from xmlSaxAttr(), but it
> just wasn't written right.  However I don't understand the intent
> of the code well enough to try to fix it.

Looks like you've stumbled on another deprecated file.  The
functionality of mod_xmlbuilder.c has been replaced by mod_xmlent.c.
I'll remove mod_xmlbuilder.c from the repository.

Thanks again,

- -bill


- --
Bill Erickson
| VP, Software Development & Integration
| Equinox Software, Inc. / The Evergreen Experts
| phone: 877-OPEN-ILS (673-6457)
| email: erickson at esilibrary.com
| web: http://esilibrary.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHjM24TYLnlSoY2kIRAhNwAKCVwmLn32EBM07GsZnBuZXyvXAD7ACfRbHo
uF8VxTHKsWDyRNMFq08m5lk=
=W2Zl
-----END PGP SIGNATURE-----