[xml] patch: xmlIO: refactor catalog handling in entity loaders

[Michael Day <mikeday yeslogic com>]

Hi,

The attached patch factors out the catalog resolution from 
xmlDefaultExternalEntityLoader and xmlNoNetExternalEntityLoader and 
places it in a separate function: xmlResolveResourceFromCatalog.

This eliminates code duplication, as both entity loader functions had 
identical catalog resolution code. It also makes it easier for people 
who are writing their own entity loader to use the existing catalog 
mechanism by calling the function.

One issue with the patch: in the original xmlIO.c, the default entity 
loader will only call the no-net entity loader within an #ifdef 
LIBXML_CATALOG_ENABLED, which seems wrong to me, as I would have thought 
that the decision to use the no-net entity loader was independent of 
whether catalogs were enabled or not. In my patch I have taken this call 
outside of the #ifdef; is this correct?

Best regards,

Michael


Index: xmlIO.c
===================================================================
RCS file: /cvs/gnome/libxml2/xmlIO.c,v
retrieving revision 1.172
diff -u -r1.172 xmlIO.c
--- xmlIO.c     1 Sep 2006 09:56:07 -0000       1.172
+++ xmlIO.c     15 Sep 2006 05:43:18 -0000
@@ -3616,6 +3616,72 @@
     return xmlCheckFilename(path);
 }

+#ifdef LIBXML_CATALOG_ENABLED
+
+xmlChar *
+xmlResolveResourceFromCatalog(const char *URL, const char *ID,
+                              xmlParserCtxtPtr ctxt) {
+    xmlChar *resource = NULL;
+    xmlCatalogAllow pref;
+
+    /*
+     * If the resource doesn't exists as a file,
+     * try to load it from the resource pointed in the catalogs
+     */
+    pref = xmlCatalogGetDefaults();
+
+    if ((pref != XML_CATA_ALLOW_NONE) && (!xmlNoNetExists(URL))) {
+       /*
+        * Do a local lookup
+        */
+       if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
+           ((pref == XML_CATA_ALLOW_ALL) ||
+            (pref == XML_CATA_ALLOW_DOCUMENT))) {
+           resource = xmlCatalogLocalResolve(ctxt->catalogs,
+                                             (const xmlChar *)ID,
+                                             (const xmlChar *)URL);
+        }
+       /*
+        * Try a global lookup
+        */
+       if ((resource == NULL) &&
+           ((pref == XML_CATA_ALLOW_ALL) ||
+            (pref == XML_CATA_ALLOW_GLOBAL))) {
+           resource = xmlCatalogResolve((const xmlChar *)ID,
+                                        (const xmlChar *)URL);
+       }
+       if ((resource == NULL) && (URL != NULL))
+           resource = xmlStrdup((const xmlChar *) URL);
+
+       /*
+        * TODO: do an URI lookup on the reference
+        */
+       if ((resource != NULL) && (!xmlNoNetExists((const char *)resource))) {
+           xmlChar *tmp = NULL;
+
+           if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
+               ((pref == XML_CATA_ALLOW_ALL) ||
+                (pref == XML_CATA_ALLOW_DOCUMENT))) {
+               tmp = xmlCatalogLocalResolveURI(ctxt->catalogs, resource);
+           }
+           if ((tmp == NULL) &&
+               ((pref == XML_CATA_ALLOW_ALL) ||
+                (pref == XML_CATA_ALLOW_GLOBAL))) {
+               tmp = xmlCatalogResolveURI(resource);
+           }
+
+           if (tmp != NULL) {
+               xmlFree(resource);
+               resource = tmp;
+           }
+       }
+    }
+
+    return resource;
+}
+
+#endif
+
 /**
  * xmlDefaultExternalEntityLoader:
  * @URL:  the URL for the entity to load
@@ -3633,15 +3699,10 @@
     xmlParserInputPtr ret = NULL;
     xmlChar *resource = NULL;

-#ifdef LIBXML_CATALOG_ENABLED
-    xmlCatalogAllow pref;
-#endif
-
 #ifdef DEBUG_EXTERNAL_ENTITIES
     xmlGenericError(xmlGenericErrorContext,
                     "xmlDefaultExternalEntityLoader(%s, xxx)\n", URL);
 #endif
-#ifdef LIBXML_CATALOG_ENABLED
     if ((ctxt != NULL) && (ctxt->options & XML_PARSE_NONET)) {
         int options = ctxt->options;

@@ -3650,60 +3711,8 @@
        ctxt->options = options;
        return(ret);
     }
-
-    /*
-     * If the resource doesn't exists as a file,
-     * try to load it from the resource pointed in the catalogs
-     */
-    pref = xmlCatalogGetDefaults();
-
-    if ((pref != XML_CATA_ALLOW_NONE) && (!xmlNoNetExists(URL))) {
-        /*
-         * Do a local lookup
-         */
-        if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
-            ((pref == XML_CATA_ALLOW_ALL) ||
-             (pref == XML_CATA_ALLOW_DOCUMENT))) {
-            resource = xmlCatalogLocalResolve(ctxt->catalogs,
-                                              (const xmlChar *) ID,
-                                              (const xmlChar *) URL);
-        }
-        /*
-         * Try a global lookup
-         */
-        if ((resource == NULL) &&
-            ((pref == XML_CATA_ALLOW_ALL) ||
-             (pref == XML_CATA_ALLOW_GLOBAL))) {
-            resource = xmlCatalogResolve((const xmlChar *) ID,
-                                         (const xmlChar *) URL);
-        }
-        if ((resource == NULL) && (URL != NULL))
-            resource = xmlStrdup((const xmlChar *) URL);
-
-        /*
-         * TODO: do an URI lookup on the reference
-         */
-        if ((resource != NULL)
-            && (!xmlNoNetExists((const char *) resource))) {
-            xmlChar *tmp = NULL;
-
-            if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
-                ((pref == XML_CATA_ALLOW_ALL) ||
-                 (pref == XML_CATA_ALLOW_DOCUMENT))) {
-                tmp = xmlCatalogLocalResolveURI(ctxt->catalogs, resource);
-            }
-            if ((tmp == NULL) &&
-                ((pref == XML_CATA_ALLOW_ALL) ||
-                 (pref == XML_CATA_ALLOW_GLOBAL))) {
-                tmp = xmlCatalogResolveURI(resource);
-            }
-
-            if (tmp != NULL) {
-                xmlFree(resource);
-                resource = tmp;
-            }
-        }
-    }
+#ifdef LIBXML_CATALOG_ENABLED
+    resource = xmlResolveResourceFromCatalog(URL, ID, ctxt);
 #endif

     if (resource == NULL)
@@ -3802,61 +3811,9 @@
     xmlChar *resource = NULL;

 #ifdef LIBXML_CATALOG_ENABLED
-    xmlCatalogAllow pref;
-
-    /*
-     * If the resource doesn't exists as a file,
-     * try to load it from the resource pointed in the catalogs
-     */
-    pref = xmlCatalogGetDefaults();
-
-    if ((pref != XML_CATA_ALLOW_NONE) && (!xmlNoNetExists(URL))) {
-       /*
-        * Do a local lookup
-        */
-       if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
-           ((pref == XML_CATA_ALLOW_ALL) ||
-            (pref == XML_CATA_ALLOW_DOCUMENT))) {
-           resource = xmlCatalogLocalResolve(ctxt->catalogs,
-                                             (const xmlChar *)ID,
-                                             (const xmlChar *)URL);
-        }
-       /*
-        * Try a global lookup
-        */
-       if ((resource == NULL) &&
-           ((pref == XML_CATA_ALLOW_ALL) ||
-            (pref == XML_CATA_ALLOW_GLOBAL))) {
-           resource = xmlCatalogResolve((const xmlChar *)ID,
-                                        (const xmlChar *)URL);
-       }
-       if ((resource == NULL) && (URL != NULL))
-           resource = xmlStrdup((const xmlChar *) URL);
-
-       /*
-        * TODO: do an URI lookup on the reference
-        */
-       if ((resource != NULL) && (!xmlNoNetExists((const char *)resource))) {
-           xmlChar *tmp = NULL;
-
-           if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
-               ((pref == XML_CATA_ALLOW_ALL) ||
-                (pref == XML_CATA_ALLOW_DOCUMENT))) {
-               tmp = xmlCatalogLocalResolveURI(ctxt->catalogs, resource);
-           }
-           if ((tmp == NULL) &&
-               ((pref == XML_CATA_ALLOW_ALL) ||
-                (pref == XML_CATA_ALLOW_GLOBAL))) {
-               tmp = xmlCatalogResolveURI(resource);
-           }
-
-           if (tmp != NULL) {
-               xmlFree(resource);
-               resource = tmp;
-           }
-       }
-    }
+    resource = xmlResolveResourceFromCatalog(URL, ID, ctxt);
 #endif
+
     if (resource == NULL)
        resource = (xmlChar *) URL;

Index: xmlIO.c
===================================================================
RCS file: /cvs/gnome/libxml2/xmlIO.c,v
retrieving revision 1.172
diff -u -r1.172 xmlIO.c
--- xmlIO.c     1 Sep 2006 09:56:07 -0000       1.172
+++ xmlIO.c     15 Sep 2006 05:43:18 -0000
@@ -3616,6 +3616,72 @@
     return xmlCheckFilename(path);
 }
 
+#ifdef LIBXML_CATALOG_ENABLED
+
+xmlChar *
+xmlResolveResourceFromCatalog(const char *URL, const char *ID,
+                              xmlParserCtxtPtr ctxt) {
+    xmlChar *resource = NULL;
+    xmlCatalogAllow pref;
+
+    /*
+     * If the resource doesn't exists as a file,
+     * try to load it from the resource pointed in the catalogs
+     */
+    pref = xmlCatalogGetDefaults();
+
+    if ((pref != XML_CATA_ALLOW_NONE) && (!xmlNoNetExists(URL))) {
+       /*
+        * Do a local lookup
+        */
+       if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
+           ((pref == XML_CATA_ALLOW_ALL) ||
+            (pref == XML_CATA_ALLOW_DOCUMENT))) {
+           resource = xmlCatalogLocalResolve(ctxt->catalogs,
+                                             (const xmlChar *)ID,
+                                             (const xmlChar *)URL);
+        }
+       /*
+        * Try a global lookup
+        */
+       if ((resource == NULL) &&
+           ((pref == XML_CATA_ALLOW_ALL) ||
+            (pref == XML_CATA_ALLOW_GLOBAL))) {
+           resource = xmlCatalogResolve((const xmlChar *)ID,
+                                        (const xmlChar *)URL);
+       }
+       if ((resource == NULL) && (URL != NULL))
+           resource = xmlStrdup((const xmlChar *) URL);
+
+       /*
+        * TODO: do an URI lookup on the reference
+        */
+       if ((resource != NULL) && (!xmlNoNetExists((const char *)resource))) {
+           xmlChar *tmp = NULL;
+
+           if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
+               ((pref == XML_CATA_ALLOW_ALL) ||
+                (pref == XML_CATA_ALLOW_DOCUMENT))) {
+               tmp = xmlCatalogLocalResolveURI(ctxt->catalogs, resource);
+           }
+           if ((tmp == NULL) &&
+               ((pref == XML_CATA_ALLOW_ALL) ||
+                (pref == XML_CATA_ALLOW_GLOBAL))) {
+               tmp = xmlCatalogResolveURI(resource);
+           }
+
+           if (tmp != NULL) {
+               xmlFree(resource);
+               resource = tmp;
+           }
+       }
+    }
+
+    return resource;
+}
+
+#endif
+
 /**
  * xmlDefaultExternalEntityLoader:
  * @URL:  the URL for the entity to load
@@ -3633,15 +3699,10 @@
     xmlParserInputPtr ret = NULL;
     xmlChar *resource = NULL;
 
-#ifdef LIBXML_CATALOG_ENABLED
-    xmlCatalogAllow pref;
-#endif
-
 #ifdef DEBUG_EXTERNAL_ENTITIES
     xmlGenericError(xmlGenericErrorContext,
                     "xmlDefaultExternalEntityLoader(%s, xxx)\n", URL);
 #endif
-#ifdef LIBXML_CATALOG_ENABLED
     if ((ctxt != NULL) && (ctxt->options & XML_PARSE_NONET)) {
         int options = ctxt->options;
 
@@ -3650,60 +3711,8 @@
        ctxt->options = options;
        return(ret);
     }
-
-    /*
-     * If the resource doesn't exists as a file,
-     * try to load it from the resource pointed in the catalogs
-     */
-    pref = xmlCatalogGetDefaults();
-
-    if ((pref != XML_CATA_ALLOW_NONE) && (!xmlNoNetExists(URL))) {
-        /*
-         * Do a local lookup
-         */
-        if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
-            ((pref == XML_CATA_ALLOW_ALL) ||
-             (pref == XML_CATA_ALLOW_DOCUMENT))) {
-            resource = xmlCatalogLocalResolve(ctxt->catalogs,
-                                              (const xmlChar *) ID,
-                                              (const xmlChar *) URL);
-        }
-        /*
-         * Try a global lookup
-         */
-        if ((resource == NULL) &&
-            ((pref == XML_CATA_ALLOW_ALL) ||
-             (pref == XML_CATA_ALLOW_GLOBAL))) {
-            resource = xmlCatalogResolve((const xmlChar *) ID,
-                                         (const xmlChar *) URL);
-        }
-        if ((resource == NULL) && (URL != NULL))
-            resource = xmlStrdup((const xmlChar *) URL);
-
-        /*
-         * TODO: do an URI lookup on the reference
-         */
-        if ((resource != NULL)
-            && (!xmlNoNetExists((const char *) resource))) {
-            xmlChar *tmp = NULL;
-
-            if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
-                ((pref == XML_CATA_ALLOW_ALL) ||
-                 (pref == XML_CATA_ALLOW_DOCUMENT))) {
-                tmp = xmlCatalogLocalResolveURI(ctxt->catalogs, resource);
-            }
-            if ((tmp == NULL) &&
-                ((pref == XML_CATA_ALLOW_ALL) ||
-                 (pref == XML_CATA_ALLOW_GLOBAL))) {
-                tmp = xmlCatalogResolveURI(resource);
-            }
-
-            if (tmp != NULL) {
-                xmlFree(resource);
-                resource = tmp;
-            }
-        }
-    }
+#ifdef LIBXML_CATALOG_ENABLED
+    resource = xmlResolveResourceFromCatalog(URL, ID, ctxt);
 #endif
 
     if (resource == NULL)
@@ -3802,61 +3811,9 @@
     xmlChar *resource = NULL;
 
 #ifdef LIBXML_CATALOG_ENABLED
-    xmlCatalogAllow pref;
-
-    /*
-     * If the resource doesn't exists as a file,
-     * try to load it from the resource pointed in the catalogs
-     */
-    pref = xmlCatalogGetDefaults();
-
-    if ((pref != XML_CATA_ALLOW_NONE) && (!xmlNoNetExists(URL))) {
-       /*
-        * Do a local lookup
-        */
-       if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
-           ((pref == XML_CATA_ALLOW_ALL) ||
-            (pref == XML_CATA_ALLOW_DOCUMENT))) {
-           resource = xmlCatalogLocalResolve(ctxt->catalogs,
-                                             (const xmlChar *)ID,
-                                             (const xmlChar *)URL);
-        }
-       /*
-        * Try a global lookup
-        */
-       if ((resource == NULL) &&
-           ((pref == XML_CATA_ALLOW_ALL) ||
-            (pref == XML_CATA_ALLOW_GLOBAL))) {
-           resource = xmlCatalogResolve((const xmlChar *)ID,
-                                        (const xmlChar *)URL);
-       }
-       if ((resource == NULL) && (URL != NULL))
-           resource = xmlStrdup((const xmlChar *) URL);
-
-       /*
-        * TODO: do an URI lookup on the reference
-        */
-       if ((resource != NULL) && (!xmlNoNetExists((const char *)resource))) {
-           xmlChar *tmp = NULL;
-
-           if ((ctxt != NULL) && (ctxt->catalogs != NULL) &&
-               ((pref == XML_CATA_ALLOW_ALL) ||
-                (pref == XML_CATA_ALLOW_DOCUMENT))) {
-               tmp = xmlCatalogLocalResolveURI(ctxt->catalogs, resource);
-           }
-           if ((tmp == NULL) &&
-               ((pref == XML_CATA_ALLOW_ALL) ||
-                (pref == XML_CATA_ALLOW_GLOBAL))) {
-               tmp = xmlCatalogResolveURI(resource);
-           }
-
-           if (tmp != NULL) {
-               xmlFree(resource);
-               resource = tmp;
-           }
-       }
-    }
+    resource = xmlResolveResourceFromCatalog(URL, ID, ctxt);
 #endif
+
     if (resource == NULL)
        resource = (xmlChar *) URL;