disable --rm on -o command

make it more similar to -c (aka `stdout`) convention.

Author: Cyan4973

programs/fileio.c

@@ -823,41 +823,62 @@ static void FIO_adjustMemLimitForPatchFromMode(FIO_prefs_t* const prefs,
     FIO_setMemLimit(prefs, (unsigned)maxSize);
 }
 
-/* FIO_removeMultiFilesWarning() :
+/* FIO_multiFilesConcatWarning() :
+ * This function handles logic when processing multiple files with -o or -c, displaying the appropriate warnings/prompts.
  * Returns 1 if the console should abort, 0 if console should proceed.
- * This function handles logic when processing multiple files with -o, displaying the appropriate warnings/prompts.
  *
- * If -f is specified, or there is just 1 file, zstd will always proceed as usual.
- * If --rm is specified, there will be a prompt asking for user confirmation.
- *         If -f is specified with --rm, zstd will proceed as usual
- *         If -q is specified with --rm, zstd will abort pre-emptively
- *         If neither flag is specified, zstd will prompt the user for confirmation to proceed.
- * If --rm is not specified, then zstd will print a warning to the user (which can be silenced with -q).
- *         Note : --rm in combination with stdout is not allowed.
+ * If output is stdout or test mode is active, check or enforce `--rm` disabled.
+ *
+ * If there is just 1 file to process, zstd will proceed as usual.
+ * If each file get processed into its own separate destination file, proceed as usual.
+ *
+ * When multiple files are processed into a single output,
+ * display a warning message, then disable --rm if it's set.
+ *
+ * If -f is specified or if output is stdout, just proceed.
+ * If output is set with -o, prompt for confirmation.
  */
-static int FIO_removeMultiFilesWarning(FIO_ctx_t* const fCtx, const FIO_prefs_t* const prefs, const char* outFileName, int displayLevelCutoff)
+static int FIO_multiFilesConcatWarning(const FIO_ctx_t* fCtx, FIO_prefs_t* prefs, const char* outFileName, int displayLevelCutoff)
 {
-    int error = 0;
-    if (fCtx->nbFilesTotal > 1 && !prefs->overwrite) {
-        if (g_display_prefs.displayLevel <= displayLevelCutoff) {
-            if (prefs->removeSrcFile) {
-                DISPLAYLEVEL(1, "zstd: Aborting... not deleting files and processing into dst: %s\n", outFileName);
-                error =  1;
-            }
-        } else {
-            if (!strcmp(outFileName, stdoutmark)) {
-                DISPLAYLEVEL(2, "zstd: WARNING: all input files will be processed and concatenated into stdout. \n");
-            } else {
-                DISPLAYLEVEL(2, "zstd: WARNING: all input files will be processed and concatenated into a single output file: %s \n", outFileName);
-            }
-            DISPLAYLEVEL(2, "The concatenated output CANNOT regenerate original file names nor directory structure. \n")
-            if (prefs->removeSrcFile) {
-                assert(fCtx->hasStdoutOutput == 0); /* not possible : never erase source files when output == stdout */
-                error = (g_display_prefs.displayLevel > displayLevelCutoff) && UTIL_requireUserConfirmation("This is a destructive operation. Proceed? (y/n): ", "Aborting...", "yY", fCtx->hasStdinInput);
-            }
-        }
+    if (fCtx->hasStdoutOutput) assert(prefs->removeSrcFile == 0);
+    if (prefs->testMode) {
+        prefs->removeSrcFile = 0;  /* enforce */
+        return 0;
     }
-    return error;
+
+    if (fCtx->nbFilesTotal == 1) return 0;
+    assert(fCtx->nbFilesTotal > 1);
+
+    if (!outFileName) return 0;
+
+    if (fCtx->hasStdoutOutput) {
+        DISPLAYLEVEL(2, "zstd: WARNING: all input files will be processed and concatenated into stdout. \n");
+    } else {
+        DISPLAYLEVEL(2, "zstd: WARNING: all input files will be processed and concatenated into a single output file: %s \n", outFileName);
+    }
+    DISPLAYLEVEL(2, "The concatenated output CANNOT regenerate original file names nor directory structure. \n")
+
+    /* multi-input into single output : --rm is not allowed */
+    if (prefs->removeSrcFile) {
+        DISPLAYLEVEL(2, "Since it's a destructive operation, input files will not be removed. \n");
+        prefs->removeSrcFile = 0;
+    }
+
+    if (fCtx->hasStdoutOutput) return 0;
+    if (prefs->overwrite) return 0;
+
+    /* multiple files concatenated into single destination file using -o without -f */
+    DISPLAY("multiple files concatenated into single destination file using -o without -f \n");
+    DISPLAY("g_display_prefs.displayLevel = %i \n", g_display_prefs.displayLevel);
+    DISPLAY("displayLevelCutoff = %i \n", displayLevelCutoff);
+    if (g_display_prefs.displayLevel <= displayLevelCutoff) {
+        /* quiet mode => no prompt => fail automatically */
+        DISPLAYLEVEL(1, "Concatenating multiple processed inputs into a single output loses file metadata. \n");
+        DISPLAYLEVEL(1, "Aborting. \n");
+        return 1;
+    }
+    /* normal mode => prompt */
+    return UTIL_requireUserConfirmation("Proceed? (y/n): ", "Aborting...", "yY", fCtx->hasStdinInput);
 }
 
 static ZSTD_inBuffer setInBuffer(const void* buf, size_t s, size_t pos)
@@ -1767,9 +1788,9 @@ FIO_compressFilename_srcFile(FIO_ctx_t* const fCtx,
             &srcFileStat, compressionLevel);
     AIO_ReadPool_closeFile(ress.readCtx);
 
-    if ( prefs->removeSrcFile   /* --rm */
-      && result == 0       /* success */
-      && strcmp(srcFileName, stdinmark)   /* exception : don't erase stdin */
+    if ( prefs->removeSrcFile  /* --rm */
+      && result == 0           /* success */
+      && strcmp(srcFileName, stdinmark)  /* exception : don't erase stdin */
       ) {
         /* We must clear the handler, since after this point calling it would
          * delete both the source and destination files.
@@ -1920,7 +1941,7 @@ int FIO_compressMultipleFilenames(FIO_ctx_t* const fCtx,
     assert(outFileName != NULL || suffix != NULL);
     if (outFileName != NULL) {   /* output into a single destination (stdout typically) */
         FILE *dstFile;
-        if (FIO_removeMultiFilesWarning(fCtx, prefs, outFileName, 1 /* displayLevelCutoff */)) {
+        if (FIO_multiFilesConcatWarning(fCtx, prefs, outFileName, 1 /* displayLevelCutoff */)) {
             FIO_freeCResources(&ress);
             return 1;
         }
@@ -2742,7 +2763,7 @@ FIO_decompressMultipleFilenames(FIO_ctx_t* const fCtx,
     dRess_t ress = FIO_createDResources(prefs, dictFileName);
 
     if (outFileName) {
-        if (FIO_removeMultiFilesWarning(fCtx, prefs, outFileName, 1 /* displayLevelCutoff */)) {
+        if (FIO_multiFilesConcatWarning(fCtx, prefs, outFileName, 1 /* displayLevelCutoff */)) {
             FIO_freeDResources(ress);
             return 1;
         }

programs/util.c

@@ -121,7 +121,7 @@ int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg,
     ch = getchar();
     result = 0;
     if (strchr(acceptableLetters, ch) == NULL) {
-        UTIL_DISPLAY("%s", abortMsg);
+        UTIL_DISPLAY("%s \n", abortMsg);
         result = 1;
     }
     /* flush the rest */

programs/zstdcli.c

@@ -1510,6 +1510,7 @@ int main(int argCount, const char* argv[])
     FIO_setHasStdoutOutput(fCtx, hasStdout);
     FIO_setNbFilesTotal(fCtx, (int)filenames->tableSize);
     FIO_determineHasStdinInput(fCtx, filenames);
+    DISPLAY("setting displayLevel to %i \n", g_displayLevel);
     FIO_setNotificationLevel(g_displayLevel);
     FIO_setAllowBlockDevices(prefs, allowBlockDevices);
     FIO_setPatchFromMode(prefs, patchFromDictFileName != NULL);

tests/playTests.sh

@@ -387,7 +387,7 @@ println "\n===>  file removal"
 zstd -f --rm tmp
 test ! -f tmp  # tmp should no longer be present
 zstd -f -d --rm tmp.zst
-test ! -f tmp.zst   # tmp.zst should no longer be present
+test ! -f tmp.zst  # tmp.zst should no longer be present
 println "test: --rm is disabled when output is stdout"
 test -f tmp
 zstd --rm tmp -c > $INTOVOID
@@ -396,6 +396,25 @@ zstd -f --rm tmp -c > $INTOVOID
 test -f tmp # tmp shall still be there
 zstd -f tmp -c > $INTOVOID --rm
 test -f tmp # tmp shall still be there
+println "test: --rm is disabled when multiple inputs are concatenated into a single output"
+cp tmp tmp2
+zstd --rm tmp tmp2 -c > $INTOVOID
+test -f tmp
+test -f tmp2
+rm -f tmp3.zst
+echo zstd tmp tmp2 -o tmp3.zst --rm with yes
+echo 'y' | zstd tmp tmp2 -o tmp3.zst --rm # prompt for confirmation
+echo operation completed succesfully after prompt
+test -f tmp
+test -f tmp2
+echo zstd -f tmp tmp2 -o tmp3.zst --rm
+zstd -f tmp tmp2 -o tmp3.zst --rm # just warns, no prompt
+test -f tmp
+test -f tmp2
+echo zstd -q tmp tmp2 -o tmp3.zst --rm
+zstd -q tmp tmp2 -o tmp3.zst --rm && die "should refuse to concatenate"
+echo operation failed as expected
+
 println "test : should quietly not remove non-regular file"
 println hello > tmp
 zstd tmp -f -o "$DEVDEVICE" 2>tmplog > "$INTOVOID"
@@ -437,34 +456,35 @@ println hello > tmp1
 println world > tmp2
 zstd tmp1 tmp2 -o "$INTOVOID" -f
 zstd tmp1 tmp2 -c | zstd -t
-zstd tmp1 tmp2 -o tmp.zst
+echo 'y' | zstd tmp1 tmp2 -o tmp.zst
 test ! -f tmp1.zst
 test ! -f tmp2.zst
 zstd tmp1 tmp2
 zstd -t tmp1.zst tmp2.zst
 zstd -dc tmp1.zst tmp2.zst
 zstd tmp1.zst tmp2.zst -o "$INTOVOID" -f
-zstd -d tmp1.zst tmp2.zst -o tmp
+echo 'y' | zstd -d tmp1.zst tmp2.zst -o tmp
 touch tmpexists
 zstd tmp1 tmp2 -f -o tmpexists
 zstd tmp1 tmp2 -q -o tmpexists && die "should have refused to overwrite"
 println gooder > tmp_rm1
 println boi > tmp_rm2
 println worldly > tmp_rm3
-echo 'y' | zstd tmp_rm1 tmp_rm2 -v -o tmp_rm3.zst --rm     # tests the warning prompt for --rm with multiple inputs into once source
-test ! -f tmp_rm1
-test ! -f tmp_rm2
+echo 'y' | zstd tmp_rm1 tmp_rm2 -v -o tmp_rm3.zst
+test -f tmp_rm1
+test -f tmp_rm2
 cp tmp_rm3.zst tmp_rm4.zst
 echo 'Y' | zstd -d tmp_rm3.zst tmp_rm4.zst -v -o tmp_rm_out --rm
-test ! -f tmp_rm3.zst
-test ! -f tmp_rm4.zst
+test -f tmp_rm3.zst
+test -f tmp_rm4.zst
 println gooder > tmpexists1
 zstd tmpexists1 tmpexists -c --rm -f > $INTOVOID
-
 # Bug: PR #972
 if [ "$?" -eq 139 ]; then
   die "should not have segfaulted"
 fi
+test -f tmpexists1
+test -f tmpexists
 println "\n===>  multiple files and shell completion "
 datagen -s1        > tmp1 2> $INTOVOID
 datagen -s2 -g100K > tmp2 2> $INTOVOID
@@ -1172,6 +1192,10 @@ zstd -t tmp3 && die "bad file not detected !"   # detects 0-sized files as bad
 println "test --rm and --test combined "
 zstd -t --rm tmp1.zst
 test -f tmp1.zst   # check file is still present
+cp tmp1.zst tmp2.zst
+zstd -t tmp1.zst tmp2.zst --rm
+test -f tmp1.zst   # check file is still present
+test -f tmp2.zst   # check file is still present
 split -b16384 tmp1.zst tmpSplit.
 zstd -t tmpSplit.* && die "bad file not detected !"
 datagen | zstd -c | zstd -t