Re: [PATCH] repack: rewrite the shell script in C.

Am 21.08.2013 10:49, schrieb Matthieu Moy:

Stefan Beller [off-list ref] writes:

quoted

+	for_each_string_list_item(item, &names) {
+		for (ext = 0; ext < 2; ext++) {
+			char *fname, *fname_old;
+			fname = mkpathdup("%s/%s%s", packdir, item->string, exts[ext]);
+			if (!file_exists(fname)) {
+				free(fname);
+				continue;
+			}
+
+			fname_old = mkpath("%s/old-%s%s", packdir, item->string, exts[ext]);
+			if (file_exists(fname_old))
+				unlink(fname_old);

Unchecked returned value.

Good catch! The original was 'rm -f ... && mv ... || failed=t'

quoted

+	/* Now the ones with the same name are out of the way... */
+	for_each_string_list_item(item, &names) {
+		for (ext = 0; ext < 2; ext++) {
+			char *fname, *fname_old;
+			struct stat statbuffer;
+			fname = mkpathdup("%s/pack-%s%s", packdir, item->string, exts[ext]);
+			fname_old = mkpath("%s-%s%s", packtmp, item->string, exts[ext]);
+			if (!stat(fname_old, &statbuffer)) {
+				statbuffer.st_mode &= ~S_IWUSR | ~S_IWGRP | ~S_IWOTH;
+				chmod(fname_old, statbuffer.st_mode);

Unchecked return value.

The original was an unchecked 'chmod a-w', so we don't care.

Of course, we could mimic the original better by issuing warnings.

quoted

+	/* Remove the "old-" files */
+	for_each_string_list_item(item, &names) {
+		char *fname;
+		fname = mkpath("%s/old-pack-%s.idx", packdir, item->string);
+		if (remove_path(fname))
+			die_errno(_("removing '%s' failed"), fname);
+
+		fname = mkpath("%s/old-pack-%s.pack", packdir, item->string);
+		if (remove_path(fname))
+			die_errno(_("removing '%s' failed"), fname);

Does this have to be a fatal error? If I read correctly, it wasn't fatal
in the shell version.

Good catch.

-- Hannes