Re: [PATCH v2 05/18] fsck: Allow demoting errors to warnings via receive.fsck.warn = <key>

Hi Junio,

On 2015-01-21 09:54, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

+	if (starts_with(var, "receive.fsck.")) {
+		if (fsck_severity.len)
+			strbuf_addch(&fsck_severity, ',');
+		strbuf_addf(&fsck_severity, "%s=%s", var + 13, value);

Wouldn't it be safer to use skip_prefix() that lets you avoid the
hardcoded "var + 13" here?

Yep, and much more elegant, too. I also fixed three more instances of the same pattern.
 

quoted

@@ -1470,8 +1478,13 @@ static const char *unpack(int err_fd, struct shallow_info *si)
 		argv_array_pushl(&child.args, "unpack-objects", hdr_arg, NULL);
 		if (quiet)
 			argv_array_push(&child.args, "-q");
-		if (fsck_objects)
-			argv_array_push(&child.args, "--strict");
+		if (fsck_objects) {
+			if (fsck_severity.len)
+				argv_array_pushf(&child.args, "--strict=%s",
+					fsck_severity.buf);
+			else
+				argv_array_push(&child.args, "--strict");
+		}
 		child.no_stdout = 1;
 		child.err = err_fd;
 		child.git_cmd = 1;

@@ -1488,8 +1501,13 @@ static const char *unpack(int err_fd, struct shallow_info *si)

 		argv_array_pushl(&child.args, "index-pack",
 				 "--stdin", hdr_arg, keep_arg, NULL);
-		if (fsck_objects)
-			argv_array_push(&child.args, "--strict");
+		if (fsck_objects) {
+			if (fsck_severity.len)
+				argv_array_pushf(&child.args, "--strict=%s",
+					fsck_severity.buf);
+			else
+				argv_array_push(&child.args, "--strict");
+		}

Hmm.  The above two hunks look suspiciously similar.  Would it be
worth to give them a single helper function?

Hmm. Not sure. I see what you mean, but for now I found

+                       argv_array_pushf(&child.args, "--strict%s%s",
+                               fsck_severity.len ? "=" : "",
+                               fsck_severity.buf);

to be more elegant than to add a fully-fledged new function. But if you feel strongly, I will gladly implement a separate function; I would appreciate suggestions as to the function name...

Ciao,
Dscho