- fix "merge" so that it properly reports need-to-use-bigger-buffer errors

- factor out some of the scarier string munging in the expand function

1 files changed, 118 insertions, 77 deletions

diff --git a/src/format.c b/src/format.c
index dc6feeb..5554788 100644
--- a/src/format.c
+++ b/src/format.c
@@ -43,11 +43,13 @@
 #include "format.h"
 #include "plugin.h"
 
+#define DEFAULT_BUFFER_SIZE 0x1000
+#define MAX_BUFFER_SIZE     0x100000
 static int format_expand(struct plugin_state *state,
 			 Slapi_PBlock *pb, Slapi_Entry *e,
 			 const char *fmt,
 			 char *outbuf, int outbuf_len,
-			 char ***visited_ndns, bool_t literal);
+			 char ***visited_ndns, bool_t contains_literals);
 
 void
 format_free_data(char *data)
@@ -110,7 +112,7 @@ format_parse_args(struct plugin_state *state, const char *args,
 	}
 	argv[argc] = NULL;
 	*out = '\0';
-	out = malloc(argc * 3 + strlen(args));
+	out = malloc((argc * 3) + strlen(args));
 	if (out != NULL) {
 		*out = '\0';
 		for (i = 0; i < argc; i++) {
@@ -392,7 +394,6 @@ format_merge(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 	     char ***visited_ndns)
 {
 	int ret, i, argc, len, slen, count;
-	char scratch[LINE_MAX];
 	char **argv;
 	ret = format_parse_args(state, args, &argc, &argv);
 	if (ret != 0) {
@@ -406,57 +407,39 @@ format_merge(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 		format_free_parsed_args(argv);
 		return -EINVAL;
 	}
+	slen = strlen(argv[0]);
 	for (i = 1, ret = 0, count = 0; i < argc; i++) {
 		/* Expand this argument. */
-		slapi_log_error(SLAPI_LOG_PLUGIN,
-				state->plugin_desc->spd_id,
-				"merge: expanding ->%s<-\n",
-				argv[i]);
+		slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,
+				"merge: expanding ->%s<-\n", argv[i]);
 		len = format_expand(state, pb, e, argv[i],
-				    scratch, sizeof(scratch),
+				    outbuf + ret + (count ? slen : 0),
+				    outbuf_len - (ret + (count ? slen : 0)),
 				    visited_ndns, TRUE);
-		/* Check that we've got space for this value. */
 		if (len < 0) {
 			slapi_log_error(SLAPI_LOG_PLUGIN,
 					state->plugin_desc->spd_id,
-					"merge: failed to expand ->%s<-: %s\n",
+					"merge: error expanding ->%s<-: %s\n",
 					argv[i], strerror(-len));
-			continue;
-		}
-		if (ret + len > outbuf_len) {
-			slapi_log_error(SLAPI_LOG_PLUGIN,
-					state->plugin_desc->spd_id,
-					"merge: out of space\n");
 			format_free_parsed_args(argv);
-			return -ENOBUFS;
+			return len;
 		}
-		slapi_log_error(SLAPI_LOG_PLUGIN,
-				state->plugin_desc->spd_id,
-				"merge: expanded ->%s<- to ->%.*s<-\n",
-				argv[i], len, scratch);
-		/* If the value is empty, skip it. */
+		/* If it didn't expand to anything, just keep going. */
 		if (len == 0) {
 			continue;
 		}
-		/* Add the separator if we need to add it. */
+		/* Log this value. */
+		slapi_log_error(SLAPI_LOG_PLUGIN,
+				state->plugin_desc->spd_id,
+				"merge: expanded ->%s<- to ->%.*s<-\n",
+				argv[i], len,
+				outbuf + ret + (count ? slen : 0));
+		/* If we filled in a result, and we had more than one in there
+		 * already, fill in the separator. */
 		if (count > 0) {
-			/* Check that we've got space for the separator. */
-			slen = strlen(argv[0]);
-			/* Check that we've got space for the separator. */
-			if (ret + len + slen > outbuf_len) {
-				slapi_log_error(SLAPI_LOG_PLUGIN,
-						state->plugin_desc->spd_id,
-						"merge: out of space\n");
-				format_free_parsed_args(argv);
-				return -ENOBUFS;
-			}
-			/* Append the separator. */
 			memcpy(outbuf + ret, argv[0], slen);
-			ret += slen;
 		}
-		/* Append the text of the expansion. */
-		memcpy(outbuf + ret, scratch, len);
-		ret += len;
+		ret += (len + (count ? slen : 0));
 		count++;
 	}
 	format_free_parsed_args(argv);
@@ -677,8 +660,8 @@ format_lookup_fn(const char *fnname)
 /* Retrieve a single value for an attribute.  If there are no values, or more
  * than one, fail. */
 static char *
-format_single(Slapi_PBlock *pb, Slapi_Entry *e, const char *attr,
-	      char ***visited_ndns)
+format_single(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
+	      const char *attr, char ***visited_ndns)
 {
 	Slapi_ValueSet *value_set;
 	Slapi_Value *value;
@@ -688,6 +671,8 @@ format_single(Slapi_PBlock *pb, Slapi_Entry *e, const char *attr,
 	if (slapi_vattr_values_get(e, (char *) attr, &value_set,
 				   &disposition, &actual_attr,
 				   0, &buffer_flags) != 0) {
+		slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,
+				"couldn't read values for \"%s\"\n", attr);
 		return NULL;
 	}
 	count = slapi_valueset_count(value_set);
@@ -699,12 +684,16 @@ format_single(Slapi_PBlock *pb, Slapi_Entry *e, const char *attr,
 				ret = strdup(cret);
 			}
 		}
+	} else {
+		slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,
+				"%d values for \"%s\"\n", count, attr);
 	}
 	slapi_vattr_values_free(&value_set, &actual_attr, buffer_flags);
 	return ret;
 }
 
-/* Find the matching closing marker. */
+/* Find the matching closing marker -- assumes that the first character in
+ * pattern is the opener. */
 static const char *
 format_find_closer(const char *pair, const char *pattern)
 {
@@ -734,6 +723,30 @@ format_find_closer(const char *pair, const char *pattern)
 	return NULL;
 }
 
+static char *
+xstrndup(const char *start, size_t length)
+{
+	char *ret;
+	ret = malloc(length + 1);
+	if (ret != NULL) {
+		memcpy(ret, start, length);
+		ret[length] = '\0';
+	}
+	return ret;
+}
+static char *
+xstrndupp(const char *start, const char *end)
+{
+	char *ret;
+	size_t length = end - start;
+	ret = malloc(length + 1);
+	if (ret != NULL) {
+		memcpy(ret, start, length);
+		ret[length] = '\0';
+	}
+	return ret;
+}
+
 /* Recursively expand the expression into the output buffer, adding any entries
  * we visit (other than e) to the list of visited NDNs.  Unless it's a literal,
  * treat the entire input format specifier as an expression. */
@@ -742,7 +755,7 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 	      const char *fmt,
 	      char *outbuf, int outbuf_len,
 	      char ***visited_ndns,
-	      bool_t literal)
+	      bool_t contains_literals)
 {
 	int i, j;
 	int exp_used, exp_size, level;
@@ -755,6 +768,7 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 		        Slapi_PBlock *pb, Slapi_Entry *e, const char *args,
 		        char *outbuf, int outbuf_len, char ***visited_ndns);
 
+	spd_id = state->plugin_desc->spd_id;
 	exp_size = outbuf_len * 2;
 	expr = malloc(exp_size);
 	if (expr == NULL) {
@@ -764,7 +778,6 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 	}
 
 	/* First, expand any subexpressions and call any "functions". */
-	spd_id = state->plugin_desc->spd_id;
 	level = 0;
 	i = 0;
 	j = 0;
@@ -797,7 +810,8 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 					 * subexpression. */
 					fmtend = match + 1;
 					/* Make a copy of the subexpression. */
-					tmp = malloc(fmtend - fmtstart);
+					tmp = xstrndupp(fmtstart + 2,
+							fmtend - 1);
 					if (tmp == NULL) {
 						slapi_log_error(SLAPI_LOG_PLUGIN,
 								spd_id,
@@ -807,9 +821,6 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 						free(expr);
 						return -ENOMEM;
 					}
-					memcpy(tmp, fmtstart + 2,
-					       fmtend - fmtstart - 3);
-					tmp[fmtend - fmtstart - 3] = '\0';
 					/* Recursively expand the
 					 * subexpression. */
 					exp_used = format_expand(state, pb, e,
@@ -825,6 +836,7 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 								spd_id,
 								"error "
 								"expanding "
+								"expression "
 								"->%s<-: %s\n",
 								tmp,
 								strerror(-exp_used));
@@ -882,7 +894,7 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 					free(expr);
 					return -EINVAL;
 				}
-				fnname = malloc(paramstart - (fmt + i));
+				fnname = xstrndupp(fmt + i + 1, paramstart);
 				if (fnname == NULL) {
 					/* Out of memory, FAIL. */
 					slapi_log_error(SLAPI_LOG_PLUGIN,
@@ -892,11 +904,8 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 					free(expr);
 					return -ENOMEM;
 				}
-				memcpy(fnname, fmt + i + 1,
-				       paramstart - (fmt + i + 1));
-				fnname[paramstart - (fmt + i + 1)] = '\0';
 				/* Isolate the parameter string. */
-				tmp = malloc(paramend - paramstart);
+				tmp = xstrndupp(paramstart + 1, paramend);
 				if (tmp == NULL) {
 					/* Out of memory, FAIL. */
 					slapi_log_error(SLAPI_LOG_PLUGIN,
@@ -907,9 +916,6 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 					free(expr);
 					return -ENOMEM;
 				}
-				memcpy(tmp, paramstart + 1,
-				       paramend - (paramstart + 1));
-				tmp[paramend - (paramstart + 1)] = '\0';
 				/* Find the "function". */
 				formatfn = format_lookup_fn(fnname);
 				if (formatfn == NULL) {
@@ -969,8 +975,8 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 	}
 	expr[j] = '\0';
 
-	if (literal) {
-		/* It's a literal string, so we're actually done. */
+	if (contains_literals) {
+		/* It's a full format specifier, so we're actually done. */
 		i = strlen(expr);
 		if (i <= outbuf_len) {
 			memcpy(outbuf, expr, i);
@@ -987,7 +993,7 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 		if (spn == strlen(expr)) {
 			/* Simple expression: expect it to be a single-valued
 			 * attribute. */
-			tmp = format_single(pb, e, expr, visited_ndns);
+			tmp = format_single(state, pb, e, expr, visited_ndns);
 			if (tmp != NULL) {
 				/* Copy the string to the output buffer if
 				 * there's space for it. */
@@ -1031,7 +1037,8 @@ format_expand(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 				break;
 			}
 			/* Retrieve the value. */
-			tmp = format_single(pb, e, attribute, visited_ndns);
+			tmp = format_single(state, pb, e,
+					    attribute, visited_ndns);
 			if (tmp == NULL) {
 				/* The attribute is undefined, or we're
 				 * treating it as if it is. */
@@ -1081,28 +1088,62 @@ static char *
 format_format(struct plugin_state *state, Slapi_PBlock *pb, Slapi_Entry *e,
 	      const char *fmt, char ***visited_ndns)
 {
-	char *buf, *tmp, *ret;
+	char *buf, *tmp, *ret, *spd_id;
 	const char *match, *fmtstart, *fmtend;
 	int i, buflen, exp_len;
 
-	buflen = state->max_value_size;
-	buf = malloc(buflen);
-	if (buf == NULL) {
-		return NULL;
-	}
-
-	i = format_expand(state, pb, e, fmt, buf, buflen, visited_ndns, TRUE);
-	if ((i >= 0) && (i < buflen)) {
-		buf[i] = '\0';
-		ret = strdup(buf);
-	} else {
-		slapi_log_error(SLAPI_LOG_PLUGIN, state->plugin_desc->spd_id,
-				"expansion of \"%s\" failed: %s\n",
-				fmt, strerror(-i));
-		ret = NULL;
-	}
+	spd_id = state->plugin_desc->spd_id;
+	buflen = DEFAULT_BUFFER_SIZE;
+	do {
+		buf = malloc(buflen);
+		if (buf == NULL) {
+			slapi_log_error(SLAPI_LOG_PLUGIN,
+					spd_id,
+					"expansion of \"%s\" "
+					"for \"%s\" failing: out of memory\n",
+					fmt,
+					slapi_entry_get_ndn(e));
+			return NULL;
+		}
 
-	free(buf);
+		i = format_expand(state, pb, e, fmt, buf, buflen,
+				  visited_ndns, TRUE);
+		if ((i >= 0) && (i < buflen)) {
+			buf[i] = '\0';
+			ret = strdup(buf);
+		} else {
+			if (i == -ENOBUFS) {
+				if (buflen < MAX_BUFFER_SIZE) {
+					buflen *= 2;
+					slapi_log_error(SLAPI_LOG_PLUGIN,
+							spd_id,
+							"expansion of \"%s\" "
+							"for \"%s\" failed: %s "
+							"(will try again)\n",
+							fmt,
+							slapi_entry_get_ndn(e),
+							strerror(-i));
+				} else {
+					slapi_log_error(SLAPI_LOG_PLUGIN,
+							spd_id,
+							"expansion of \"%s\" "
+							"for \"%s\" failed: %s "
+							"(giving up)\n",
+							fmt,
+							slapi_entry_get_ndn(e),
+							strerror(-i));
+				}
+			} else {
+				slapi_log_error(SLAPI_LOG_PLUGIN, spd_id,
+						"expansion of \"%s\" "
+						"for \"%s\" failed: %s\n",
+						fmt, slapi_entry_get_ndn(e),
+						strerror(-i));
+			}
+			ret = NULL;
+		}
+		free(buf);
+	} while (i == -ENOBUFS);
 
 	return ret;
 }