In article <yc+ztnkigmts1...@homeworld.netbsd.org>,
nia <n...@netbsd.org> 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;
>
