Fix mask path placeholder check

The mask paths for all panels have to have the same number of
placeholders, but the masks do not have to have the same number of
placeholders as the panel data blocks.

This also tidies up a few excess strdup() calls, and removes
partial_event_substitution() because retrieve_full_path() can now handle
the number of placeholders being too small.

4 files changed, 43 insertions, 39 deletions

diff --git a/libcrystfel/src/detector.c b/libcrystfel/src/detector.c
index 42ffbe40..8c98f79e 100644
--- a/libcrystfel/src/detector.c
+++ b/libcrystfel/src/detector.c
@@ -1204,7 +1204,7 @@ struct detector *get_detector_geometry_2(const char *filename,
 	int i;
 	int rgi, rgci;
 	int reject = 0;
-	int path_dim;
+	int path_dim, mask_path_dim;
 	int dim_dim;
 	int dim_reject = 0;
 	int dim_dim_reject = 0;
@@ -1389,6 +1389,7 @@ struct detector *get_detector_geometry_2(const char *filename,
 
 	}
 
+	mask_path_dim = -1;
 	for ( i=0; i<det->n_panels; i++ ) {
 
 		int panel_mask_dim = 0;
@@ -1398,8 +1399,7 @@ struct detector *get_detector_geometry_2(const char *filename,
 
 			next_instance = det->panels[i].mask;
 
-			while(next_instance)
-			{
+			while ( next_instance ) {
 				next_instance = strstr(next_instance, "%");
 				if ( next_instance != NULL ) {
 					next_instance += 1*sizeof(char);
@@ -1407,18 +1407,29 @@ struct detector *get_detector_geometry_2(const char *filename,
 				}
 			}
 
-			if ( panel_mask_dim != path_dim ) {
-				dim_reject = 1;
+			if ( mask_path_dim == -1 ) {
+				mask_path_dim = panel_mask_dim;
+			} else {
+				if ( panel_mask_dim != mask_path_dim ) {
+					dim_reject = 1;
+				}
 			}
+
 		}
 	}
 
-	if ( dim_reject ==  1) {
+	if ( dim_reject ==  1 ) {
 		ERROR("All panels' data and mask entries must have the same "
 		      "number of placeholders\n");
 		reject = 1;
 	}
 
+	if ( mask_path_dim > path_dim ) {
+		ERROR("Number of placeholders in mask cannot be larger than "
+		      "for data\n");
+		reject = 1;
+	}
+
 	det->path_dim = path_dim;
 
 	dim_dim_reject = 0;
diff --git a/libcrystfel/src/events.c b/libcrystfel/src/events.c
index 25771a69..0f170bb5 100644
--- a/libcrystfel/src/events.c
+++ b/libcrystfel/src/events.c
@@ -3,10 +3,11 @@
  *
  * Event properties
  *
- * Copyright © 2012-2014 Deutsches Elektronen-Synchrotron DESY,
+ * Copyright © 2012-2017 Deutsches Elektronen-Synchrotron DESY,
  *                       a research centre of the Helmholtz Association.
  *
  * Authors:
+ *   2017      Thomas White
  *   2014      Valerio Mariani
  *
  * This file is part of CrystFEL.
@@ -30,6 +31,7 @@
 #define _GNU_SOURCE
 
 #include "events.h"
+#include "utils.h"
 
 #include <hdf5.h>
 #include <string.h>
@@ -585,46 +587,38 @@ char *retrieve_full_path(struct event *ev, const char *data)
 {
 	int ei ;
 	char *return_value;
+	char *pholder;
 
 	return_value = strdup(data);
+	pholder = strstr(return_value,"%");
+	ei = 0;
 
-	for ( ei=0; ei<ev->path_length; ei++ ) {
+	while ( pholder != NULL ) {
 
 		char *tmp;
-		tmp = event_path_placeholder_subst(ev->path_entries[ei],
-		                                   return_value);
-
-		free(return_value);
-		return_value = tmp;
-
-	}
-
-	return return_value;
-
-}
-
-
-char *partial_event_substitution(struct event *ev, const char *data)
-{
-	int ei ;
-	char *return_value;
-	char *pholder;
 
-	return_value = strdup(data);
-	pholder = strstr(return_value,"%");
-	ei = 0;
+		/* Check we have enough things to put in the placeholders */
+		if ( ei >= ev->path_length ) {
+			ERROR("Too many placeholders ('%%') in location.\n");
+			return NULL;
+		}
 
-	while( pholder != NULL) {
+		/* Substitute one placeholder */
+		tmp = event_path_placeholder_subst(ev->path_entries[ei++],
+		                                         return_value);
 
-		char *tmp_subst_data;
+		if ( tmp == NULL ) {
+			ERROR("Couldn't substitute placeholder\n");
+			return NULL;
+		}
 
-		tmp_subst_data = event_path_placeholder_subst(ev->path_entries[ei],
-                                                      return_value);
+		/* Next time, we will substitute the next part of the path into
+		 * the partially substituted string */
 		free(return_value);
-		return_value = strdup(tmp_subst_data);
-		free(tmp_subst_data);
+		return_value = tmp;
+
 		pholder = strstr(return_value, "%");
-		ei += 1;
+
 	}
 
 	return return_value;
diff --git a/libcrystfel/src/events.h b/libcrystfel/src/events.h
index 7f9c6731..4f717209 100644
--- a/libcrystfel/src/events.h
+++ b/libcrystfel/src/events.h
@@ -78,7 +78,6 @@ extern char *get_event_string(struct event *ev);
 extern struct event *get_event_from_event_string(const char *ev_string);
 extern char *event_path_placeholder_subst(const char *ev_name,
                                           const char *data);
-extern char *partial_event_substitution(struct event *ev, const char *data);
 extern char *retrieve_full_path(struct event *ev, const char *data);
 
 
diff --git a/libcrystfel/src/hdf5-file.c b/libcrystfel/src/hdf5-file.c
index 6542b360..9bcdb1b7 100644
--- a/libcrystfel/src/hdf5-file.c
+++ b/libcrystfel/src/hdf5-file.c
@@ -1154,7 +1154,7 @@ static int get_ev_based_value(struct hdfile *f, const char *name,
 	char *subst_name = NULL;
 
 	if ( ev->path_length != 0 ) {
-		subst_name = partial_event_substitution(ev, name);
+		subst_name = retrieve_full_path(ev, name);
 	} else {
 		subst_name = strdup(name);
 	}
@@ -1966,7 +1966,7 @@ char *hdfile_get_string_value(struct hdfile *f, const char *name,
 	char *tmp = NULL, *subst_name = NULL;
 
 	if (ev != NULL && ev->path_length != 0 ) {
-		subst_name = partial_event_substitution(ev, name);
+		subst_name = retrieve_full_path(ev, name);
 	} else {
 		subst_name = strdup(name);
 	}