Fix an error in xpath_get_string with empty/unset elements

engine/drumkit.c

@@ -94,10 +94,9 @@ static void xpath_get_float(float *out, xmlNodePtr node, const char *xpathExpr,
   const char *s;
 
   s = xpath_value_as_string(node, xpathExpr, xpathCtx);
-  if (s == NULL) {
-    return;
+  if (s) {
+    *out = strtof(s, NULL);
   }
-  *out = strtof(s, NULL);
 }
 
 // Evaluate an XPath expression and store the result as an
@@ -108,10 +107,9 @@ static void xpath_get_int(int *out, xmlNodePtr node, const char *xpathExpr,
   const char *s;
 
   s = xpath_value_as_string(node, xpathExpr, xpathCtx);
-  if (s == NULL) {
-    return;
+  if (s) {
+    *out = atoi(s);
   }
-  *out = atoi(s);
 }
 
 // Evaluate an XPath expression and store the result as a string. The
@@ -120,7 +118,12 @@ static void xpath_get_int(int *out, xmlNodePtr node, const char *xpathExpr,
 static void xpath_get_string(char **out, xmlNodePtr node,
                              const char *xpathExpr,
                              xmlXPathContextPtr xpathCtx) {
-  *out = strdup(xpath_value_as_string(node, xpathExpr, xpathCtx));
+  const char *s;
+
+  s = xpath_value_as_string(node, xpathExpr, xpathCtx);
+  if (s) {
+    *out = strdup(s);
+  }
 }
 
 // Count the number of nodes returned by an XPath expression.
@@ -300,17 +303,19 @@ static int parse_instrument(struct instrument *ins, xmlNodePtr node,
 
   layers = xmlXPathNodeEval(node, (xmlChar *)".//dk:layer", xpathCtx);
   if (layers == NULL || layers->nodesetval == NULL) {
-    printf("error: instrument has no layers?\n");
+    fprintf(stderr, "warning: instrument has no layers\n");
     goto fail;
   }
 
   ins->num_layers = layers->nodesetval->nodeNr;
-  ins->layers = (struct layer *)calloc(ins->num_layers, sizeof(struct layer));
-  for (i = 0; i < ins->num_layers; i++) {
-    if (!parse_layer(&ins->layers[i], layers->nodesetval->nodeTab[i],
-                     xpathCtx, basedir, target_samplerate)) {
-      printf("error parsing instrument layer %d\n", i);
-      goto fail;
+  if (ins->num_layers > 0) {
+    ins->layers = (struct layer *)calloc(ins->num_layers, sizeof(struct layer));
+    for (i = 0; i < ins->num_layers; i++) {
+      if (!parse_layer(&ins->layers[i], layers->nodesetval->nodeTab[i],
+                       xpathCtx, basedir, target_samplerate)) {
+        fprintf(stderr, "error parsing instrument layer %d\n", i);
+        goto fail;
+      }
     }
   }
 
@@ -327,8 +332,10 @@ static void free_instrument(struct instrument *ins) {
   for (i = 0; i < ins->num_layers; i++) {
     free_layer(&ins->layers[i]);
   }
-  free(ins->layers);
-  free(ins->name);
+  if (ins->layers)
+    free(ins->layers);
+  if (ins->name)
+    free(ins->name);
 }
 
 // Find the instrument layer corresponding to a specific velocity.
@@ -350,7 +357,6 @@ static int parse_instruments(struct instrument **instruments,
   int i, n;
 
   n = nodes->nodeNr;
-  printf("found %d instruments\n", n);
   *instruments = (struct instrument *)calloc(n, sizeof(struct instrument));
 
   for (i = 0; i < n; i++) {
@@ -389,7 +395,7 @@ static void fix_midi_notes(struct drumkit *kit) {
 
   // Assign default MIDI note mapping if all the notes are the same.
   if (note_cardinality == 1) {
-    printf("drumkit lacks proper MIDI note mapping, setting default assignments...\n");
+    fprintf(stderr, "warning: drumkit lacks proper MIDI note mapping, setting default assignments\n");
     for (i = 0; i < kit->num_instruments; i++) {
       kit->instruments[i].note = i + 36;
     }
@@ -406,9 +412,12 @@ struct drumkit *drumkit_new(const char *filename, int target_samplerate) {
 }
 
 // Assign the Hydrogen drumkit namespace to all XML nodes in a document.
+//
+// BUG: this causes a memory leak of a few bytes per XML node,
+// reported by the address sanitizer.
 static void fix_bad_xml(xmlNodePtr cur, xmlNsPtr ns) {
   for (; cur; cur = cur->next) {
-    if (cur->type == XML_ELEMENT_NODE) {
+    if (cur->type == XML_ELEMENT_NODE && !cur->ns) {
       xmlSetNs(cur, ns);
     }
     fix_bad_xml(cur->children, ns);
@@ -418,6 +427,7 @@ static void fix_bad_xml(xmlNodePtr cur, xmlNsPtr ns) {
 int drumkit_load(struct drumkit *kit, const char *filename, int target_samplerate) {
   char *basedir = NULL;
   xmlDocPtr doc = NULL;
+  xmlNsPtr ns = NULL;
   xmlNodePtr root_element = NULL;
   xmlXPathContextPtr xpathCtx = NULL;
   xmlXPathObjectPtr xpathObj = NULL;
@@ -438,7 +448,6 @@ int drumkit_load(struct drumkit *kit, const char *filename, int target_samplerat
   // elements.
   root_element = xmlDocGetRootElement(doc);
   if (!strcmp(root_element->name, "drumkit_info") && root_element->ns == NULL) {
-    xmlNsPtr ns;
     fprintf(stderr, "spotted old-style (bad) XML\n");
     ns = xmlNewNs(root_element, (xmlChar *)"http://www.hydrogen-music.org/drumkit", (xmlChar *)"");
     fix_bad_xml(root_element, ns);

engine/sampler.c

@@ -42,7 +42,7 @@ void sampler_noteon(struct sampler *sampler, int note, int velocity) {
   
   ins = drumkit_find_instrument_by_note(sampler->kit, note);
   if (!ins) {
-    printf("no instrument found for note %d\n", note);
+    fprintf(stderr, "no instrument found for note %d\n", note);
     return;
   }
 
@@ -62,7 +62,7 @@ void sampler_noteon(struct sampler *sampler, int note, int velocity) {
     }
   }
   if (!voice) {
-    printf("all voices are busy\n");
+    fprintf(stderr, "all voices are busy\n");
     return;
   }