Re: patch: Option to not create backup files creates backup files

[christos%astron.com@localhost (Christos Zoulas)]

In article <YC+zTnkIGmtS10GZ%homeworld.netbsd.org@localhost>,
nia  <nia%NetBSD.org@localhost> wrote:
>I noticed some odd behaviour when I tried to use patch(1) without
>creating backup files (-V none). It seems this option is ignored,
>and backup files are created anyway. The PATCH_VERSION_CONTROL
>environment variable can also override -V none, which is the
>opposite of the behaviour stated in the man page.
>
>The problem in the code seems to be that patch treats "none"
>as the same as "undefined" (or "no option specified"), and
>freely allows it to be overriden.
>
>If there's no PATCH_VERSION_CONTROL environment variable and
>the backup type is undefined ("none") after the command line
>options are parsed, NULL is passed to get_version, which results
>in numbered_existing being returned, making it impossible to
>use -V none.
>
>This diff introduces an "undefined" backup type and uses it
>in places "none" was previously mistakenly used, allowing
>"none" to be passed with -V and work as expected.
>
>Please review.

LGTM.

christos
>
>Index: patch/backupfile.c
>===================================================================
>RCS file: /cvsroot/src/usr.bin/patch/backupfile.c,v
>retrieving revision 1.15
>diff -u -r1.15 backupfile.c
>--- patch/backupfile.c	11 Apr 2014 17:30:03 -0000	1.15
>+++ patch/backupfile.c	19 Feb 2021 12:39:45 -0000
>@@ -38,7 +38,7 @@
> #define ISDIGIT(c) (isascii ((unsigned char)c) && isdigit ((unsigned char)c))
> 
> /* Which type of backup file names are generated. */
>-enum backup_type backup_type = none;
>+enum backup_type backup_type = undefined;
> 
> /*
>  * The extension added to file names to produce a simple (as opposed to
>Index: patch/backupfile.h
>===================================================================
>RCS file: /cvsroot/src/usr.bin/patch/backupfile.h,v
>retrieving revision 1.6
>diff -u -r1.6 backupfile.h
>--- patch/backupfile.h	19 Sep 2008 18:33:34 -0000	1.6
>+++ patch/backupfile.h	19 Feb 2021 12:39:45 -0000
>@@ -19,6 +19,8 @@
> 
> /* When to make backup files. */
> enum backup_type {
>+	undefined,
>+
> 	/* Never make backups. */
> 	none,
> 
>Index: patch/patch.c
>===================================================================
>RCS file: /cvsroot/src/usr.bin/patch/patch.c,v
>retrieving revision 1.29
>diff -u -r1.29 patch.c
>--- patch/patch.c	6 Sep 2011 18:25:14 -0000	1.29
>+++ patch/patch.c	19 Feb 2021 12:39:45 -0000
>@@ -197,17 +197,18 @@
> 	else
> 		simple_backup_suffix = ORIGEXT;
> 
>+	if ((v = getenv("PATCH_VERSION_CONTROL")) == NULL)
>+		v = getenv("VERSION_CONTROL");
>+	if (v != NULL)
>+		backup_type = get_version(v);
>+
> 	/* parse switches */
> 	Argc = argc;
> 	Argv = argv;
> 	get_some_switches();
> 
>-	if (backup_type == none) {
>-		if ((v = getenv("PATCH_VERSION_CONTROL")) == NULL)
>-			v = getenv("VERSION_CONTROL");
>-		if (v != NULL || !posix)
>-			backup_type = get_version(v);	/* OK to pass NULL. */
>-	}
>+	if (backup_type == undefined)
>+		backup_type = posix ? none : numbered_existing;
> 
> 	/* make sure we clean up /tmp in case of disaster */
> 	set_signals(0);
>@@ -493,7 +494,7 @@
> 	while ((ch = getopt_long(Argc, Argv, options, longopts, NULL)) != -1) {
> 		switch (ch) {
> 		case 'b':
>-			if (backup_type == none)
>+			if (backup_type == undefined)
> 				backup_type = numbered_existing;
> 			if (optarg == NULL)
> 				break;
>