disable --rm on -o command

Cyan4973 · Cyan4973 · commit 75a7497736ac · 2023-01-24T16:39:24.000-08:00
make it more similar to -c (aka `stdout`) convention.
diff --git a/programs/fileio.c b/programs/fileio.c
@@ -612,7 +612,7 @@ FIO_openDstFile(FIO_ctx_t* fCtx, FIO_prefs_t* const prefs,
         if (!prefs->overwrite) {
             if (g_display_prefs.displayLevel <= 1) {
                 /* No interaction possible */
-                DISPLAY("zstd: %s already exists; not overwritten  \n",
+                DISPLAYLEVEL(1, "zstd: %s already exists; not overwritten  \n",
                         dstFileName);
                 return NULL;
             }
@@ -723,7 +723,7 @@ int FIO_checkFilenameCollisions(const char** filenameTable, unsigned nbFiles) {
 
     filenameTableSorted = (const char**) malloc(sizeof(char*) * nbFiles);
     if (!filenameTableSorted) {
-        DISPLAY("Unable to malloc new str array, not checking for name collisions\n");
+        DISPLAYLEVEL(1, "Allocation error during filename collision checking \n");
         return 1;
     }
 
@@ -740,7 +740,7 @@ int FIO_checkFilenameCollisions(const char** filenameTable, unsigned nbFiles) {
     prevElem = filenameTableSorted[0];
     for (u = 1; u < nbFiles; ++u) {
         if (strcmp(prevElem, filenameTableSorted[u]) == 0) {
-            DISPLAY("WARNING: Two files have same filename: %s\n", prevElem);
+            DISPLAYLEVEL(2, "WARNING: Two files have same filename: %s\n", prevElem);
         }
         prevElem = filenameTableSorted[u];
     }
@@ -823,41 +823,59 @@ 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 that `--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) {
+        assert(prefs->removeSrcFile == 0);
+        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 */
+    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 +1785,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.
@@ -1791,7 +1809,8 @@ checked_index(const char* options[], size_t length, size_t index) {
 
 #define INDEX(options, index) checked_index((options), sizeof(options)  / sizeof(char*), (size_t)(index))
 
-void FIO_displayCompressionParameters(const FIO_prefs_t* prefs) {
+void FIO_displayCompressionParameters(const FIO_prefs_t* prefs)
+{
     static const char* formatOptions[5] = {ZSTD_EXTENSION, GZ_EXTENSION, XZ_EXTENSION,
                                            LZMA_EXTENSION, LZ4_EXTENSION};
     static const char* sparseOptions[3] = {" --no-sparse", "", " --sparse"};
@@ -1920,7 +1939,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 +2761,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;
         }
diff --git a/programs/util.c b/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 */
diff --git a/programs/zstdcli.c b/programs/zstdcli.c
@@ -339,7 +339,7 @@ static const char* lastNameFromPath(const char* path)
 
 static void errorOut(const char* msg)
 {
-    DISPLAY("%s \n", msg); exit(1);
+    DISPLAYLEVEL(1, "%s \n", msg); exit(1);
 }
 
 /*! readU32FromCharChecked() :
@@ -786,13 +786,13 @@ static unsigned init_nbThreads(void) {
     } else {                      \
         argNb++;                  \
         if (argNb >= argCount) {  \
-            DISPLAY("error: missing command argument \n"); \
+            DISPLAYLEVEL(1, "error: missing command argument \n"); \
             CLEAN_RETURN(1);      \
         }                         \
         ptr = argv[argNb];        \
         assert(ptr != NULL);      \
         if (ptr[0]=='-') {        \
-            DISPLAY("error: command cannot be separated from its argument by another command \n"); \
+            DISPLAYLEVEL(1, "error: command cannot be separated from its argument by another command \n"); \
             CLEAN_RETURN(1);      \
 }   }   }
 
@@ -859,6 +859,7 @@ int main(int argCount, const char* argv[])
 
     FIO_prefs_t* const prefs = FIO_createPreferences();
     FIO_ctx_t* const fCtx = FIO_createContext();
+    FIO_progressSetting_e progress = FIO_ps_auto;
     zstd_operation_mode operation = zom_compress;
     ZSTD_compressionParameters compressionParams;
     int cLevel = init_cLevel();
@@ -898,7 +899,7 @@ int main(int argCount, const char* argv[])
     (void)recursive; (void)cLevelLast;    /* not used when ZSTD_NOBENCH set */
     (void)memLimit;
     assert(argCount >= 1);
-    if ((filenames==NULL) || (file_of_names==NULL)) { DISPLAY("zstd: allocation error \n"); exit(1); }
+    if ((filenames==NULL) || (file_of_names==NULL)) { DISPLAYLEVEL(1, "zstd: allocation error \n"); exit(1); }
     programName = lastNameFromPath(programName);
 #ifdef ZSTD_MULTITHREAD
     nbWorkers = init_nbThreads();
@@ -999,8 +1000,8 @@ int main(int argCount, const char* argv[])
                 if (!strcmp(argument, "--rsyncable")) { rsyncable = 1; continue; }
                 if (!strcmp(argument, "--compress-literals")) { literalCompressionMode = ZSTD_ps_enable; continue; }
                 if (!strcmp(argument, "--no-compress-literals")) { literalCompressionMode = ZSTD_ps_disable; continue; }
-                if (!strcmp(argument, "--no-progress")) { FIO_setProgressSetting(FIO_ps_never); continue; }
-                if (!strcmp(argument, "--progress")) { FIO_setProgressSetting(FIO_ps_always); continue; }
+                if (!strcmp(argument, "--no-progress")) { progress = FIO_ps_never; continue; }
+                if (!strcmp(argument, "--progress")) { progress = FIO_ps_always; continue; }
                 if (!strcmp(argument, "--exclude-compressed")) { FIO_setExcludeCompressedFile(prefs, 1); continue; }
                 if (!strcmp(argument, "--fake-stdin-is-console")) { UTIL_fakeStdinIsConsole(); continue; }
                 if (!strcmp(argument, "--fake-stdout-is-console")) { UTIL_fakeStdoutIsConsole(); continue; }
@@ -1057,7 +1058,7 @@ int main(int argCount, const char* argv[])
                 if (longCommandWArg(&argument, "--output-dir-flat")) {
                     NEXT_FIELD(outDirName);
                     if (strlen(outDirName) == 0) {
-                        DISPLAY("error: output dir cannot be empty string (did you mean to pass '.' instead?)\n");
+                        DISPLAYLEVEL(1, "error: output dir cannot be empty string (did you mean to pass '.' instead?)\n");
                         CLEAN_RETURN(1);
                     }
                     continue;
@@ -1073,7 +1074,7 @@ int main(int argCount, const char* argv[])
                 if (longCommandWArg(&argument, "--output-dir-mirror")) {
                     NEXT_FIELD(outMirroredDirName);
                     if (strlen(outMirroredDirName) == 0) {
-                        DISPLAY("error: output dir cannot be empty string (did you mean to pass '.' instead?)\n");
+                        DISPLAYLEVEL(1, "error: output dir cannot be empty string (did you mean to pass '.' instead?)\n");
                         CLEAN_RETURN(1);
                     }
                     continue;
@@ -1349,7 +1350,7 @@ int main(int argCount, const char* argv[])
         int const ret = FIO_listMultipleFiles((unsigned)filenames->tableSize, filenames->fileNames, g_displayLevel);
         CLEAN_RETURN(ret);
 #else
-        DISPLAY("file information is not supported \n");
+        DISPLAYLEVEL(1, "file information is not supported \n");
         CLEAN_RETURN(1);
 #endif
     }
@@ -1480,24 +1481,29 @@ int main(int argCount, const char* argv[])
 
     if (showDefaultCParams) {
         if (operation == zom_decompress) {
-            DISPLAY("error : can't use --show-default-cparams in decompression mode \n");
+            DISPLAYLEVEL(1, "error : can't use --show-default-cparams in decompression mode \n");
             CLEAN_RETURN(1);
         }
     }
 
     if (dictFileName != NULL && patchFromDictFileName != NULL) {
-        DISPLAY("error : can't use -D and --patch-from=# at the same time \n");
+        DISPLAYLEVEL(1, "error : can't use -D and --patch-from=# at the same time \n");
         CLEAN_RETURN(1);
     }
 
     if (patchFromDictFileName != NULL && filenames->tableSize > 1) {
-        DISPLAY("error : can't use --patch-from=# on multiple files \n");
+        DISPLAYLEVEL(1, "error : can't use --patch-from=# on multiple files \n");
         CLEAN_RETURN(1);
     }
 
-    /* No status message in pipe mode (stdin - stdout) */
+    /* No status message by default when output is stdout */
     hasStdout = outFileName && !strcmp(outFileName,stdoutmark);
-    if ((hasStdout || !UTIL_isConsole(stderr)) && (g_displayLevel==2)) g_displayLevel=1;
+    if (hasStdout && (g_displayLevel==2)) g_displayLevel=1;
+
+    /* when stderr is not the console, do not pollute it with status updates
+     * Note : the below code actually also silence more stuff, including completion report. */
+    if (!UTIL_isConsole(stderr) && (g_displayLevel==2)) g_displayLevel=1;
+    FIO_setProgressSetting(progress);
 
     /* don't remove source files when output is stdout */;
     if (hasStdout && removeSrcFile) {
@@ -1569,7 +1575,7 @@ int main(int argCount, const char* argv[])
             operationResult = FIO_compressMultipleFilenames(fCtx, prefs, filenames->fileNames, outMirroredDirName, outDirName, outFileName, suffix, dictFileName, cLevel, compressionParams);
 #else
         (void)contentSize; (void)suffix; (void)adapt; (void)rsyncable; (void)ultra; (void)cLevel; (void)ldmFlag; (void)literalCompressionMode; (void)targetCBlockSize; (void)streamSrcSize; (void)srcSizeHint; (void)ZSTD_strategyMap; (void)useRowMatchFinder; /* not used when ZSTD_NOCOMPRESS set */
-        DISPLAY("Compression not supported \n");
+        DISPLAYLEVEL(1, "Compression not supported \n");
 #endif
     } else {  /* decompression or test */
 #ifndef ZSTD_NODECOMPRESS
@@ -1579,7 +1585,7 @@ int main(int argCount, const char* argv[])
             operationResult = FIO_decompressMultipleFilenames(fCtx, prefs, filenames->fileNames, outMirroredDirName, outDirName, outFileName, dictFileName);
         }
 #else
-        DISPLAY("Decompression not supported \n");
+        DISPLAYLEVEL(1, "Decompression not supported \n");
 #endif
     }
 
diff --git a/tests/playTests.sh b/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,20 @@ 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 'y' | zstd -v tmp tmp2 -o tmp3.zst --rm # prompt for confirmation
+test -f tmp
+test -f tmp2
+zstd -f tmp tmp2 -o tmp3.zst --rm # just warns, no prompt
+test -f tmp
+test -f tmp2
+zstd -q tmp tmp2 -o tmp3.zst --rm && die "should refuse to concatenate"
+
 println "test : should quietly not remove non-regular file"
 println hello > tmp
 zstd tmp -f -o "$DEVDEVICE" 2>tmplog > "$INTOVOID"
@@ -437,34 +451,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 -v 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 -v -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 -v 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
+echo 'Y' | zstd -v -d tmp_rm3.zst tmp_rm4.zst -v -o tmp_rm_out --rm
+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 +1187,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

Original file line number	Diff line number	Diff line change
`@@ -121,7 +121,7 @@ int UTIL_requireUserConfirmation(const char* prompt, const char* abortMsg,`
`121`	`121`	`ch = getchar();`
`122`	`122`	`result = 0;`
`123`	`123`	`if (strchr(acceptableLetters, ch) == NULL) {`
`124`		`- UTIL_DISPLAY("%s", abortMsg);`
	`124`	`+ UTIL_DISPLAY("%s \n", abortMsg);`
`125`	`125`	`result = 1;`
`126`	`126`	`}`
`127`	`127`	`/* flush the rest */`