From 4c16608e3cb1fae8008466cc8684b8ea0e1d8449 Mon Sep 17 00:00:00 2001 From: Topher Lubaway Date: Mon, 11 Jun 2018 10:13:00 -0700 Subject: [PATCH 1/8] Improves UX for --list command's lack of support for pipes --list does not support piped input This checks for a terminal and exits 1 with a well formatted error message if the STDIN is not from a terminal --- programs/fileio.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/programs/fileio.c b/programs/fileio.c index b1d2a241..a9833ae5 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -27,6 +27,7 @@ #include "platform.h" /* Large Files support, SET_BINARY_MODE */ #include "util.h" /* UTIL_getFileSize, UTIL_isRegularFile */ #include /* fprintf, fopen, fread, _fileno, stdin, stdout */ +#include /* isatty */ #include /* malloc, free */ #include /* strcmp, strlen */ #include /* errno */ @@ -2030,6 +2031,10 @@ static int FIO_listFile(fileInfo_t* total, const char* inFileName, int displayLe } int FIO_listMultipleFiles(unsigned numFiles, const char** filenameTable, int displayLevel){ + if (!isatty(0)) + DISPLAYOUT("zstd --list does not support reading from standard input\n"); + return 1; + if (numFiles == 0) { DISPLAYOUT("No files given\n"); return 0; From 5ca1d5c6f445d3c3240132eb4a42c6eb50882cab Mon Sep 17 00:00:00 2001 From: Topher Lubaway Date: Mon, 11 Jun 2018 12:19:15 -0700 Subject: [PATCH 2/8] Properly brackets isatty if statement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ¯\_(ツ)_/¯ this is my first commit in c --- programs/fileio.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index a9833ae5..0cfc968f 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -2031,9 +2031,10 @@ static int FIO_listFile(fileInfo_t* total, const char* inFileName, int displayLe } int FIO_listMultipleFiles(unsigned numFiles, const char** filenameTable, int displayLevel){ - if (!isatty(0)) - DISPLAYOUT("zstd --list does not support reading from standard input\n"); + if (!isatty(0)) { + DISPLAYOUT("zstd: --list does not support reading from standard input\n"); return 1; + } if (numFiles == 0) { DISPLAYOUT("No files given\n"); From 881defaeb39e20ad2b11ff1c07237e58c644739e Mon Sep 17 00:00:00 2001 From: Topher Lubaway Date: Mon, 11 Jun 2018 15:26:35 -0700 Subject: [PATCH 3/8] Only check for tty in non-windows environments unistd.h is for unix standard tools. There does not appear to be a simple isatty for windows this we only run the logic and header include in non-windows environments --- programs/fileio.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/programs/fileio.c b/programs/fileio.c index 0cfc968f..df68b235 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -27,7 +27,6 @@ #include "platform.h" /* Large Files support, SET_BINARY_MODE */ #include "util.h" /* UTIL_getFileSize, UTIL_isRegularFile */ #include /* fprintf, fopen, fread, _fileno, stdin, stdout */ -#include /* isatty */ #include /* malloc, free */ #include /* strcmp, strlen */ #include /* errno */ @@ -35,6 +34,8 @@ #if defined (_MSC_VER) # include # include +#else +# include /* isatty */ #endif #include "mem.h" @@ -2031,10 +2032,14 @@ static int FIO_listFile(fileInfo_t* total, const char* inFileName, int displayLe } int FIO_listMultipleFiles(unsigned numFiles, const char** filenameTable, int displayLevel){ + // isatty comes from the header unistd.h + // windows has no equilivant + #if !defined(_MSC_VER) if (!isatty(0)) { DISPLAYOUT("zstd: --list does not support reading from standard input\n"); return 1; } + #endif if (numFiles == 0) { DISPLAYOUT("No files given\n"); From 88ae51acb39f2ce1bd0abc9707ea1279ce2987dc Mon Sep 17 00:00:00 2001 From: Topher Lubaway Date: Tue, 12 Jun 2018 07:59:17 -0700 Subject: [PATCH 4/8] Multi-OS support for --list detecting stream input IS_CONSOLE stolen wholesale from Options.cpp not sure if i should have extracted that code for DRY-ness tested in OSX and functionality seems appropriate unstested in a windows environment --- programs/fileio.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index df68b235..d33c7f42 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -31,13 +31,6 @@ #include /* strcmp, strlen */ #include /* errno */ -#if defined (_MSC_VER) -# include -# include -#else -# include /* isatty */ -#endif - #include "mem.h" #include "fileio.h" #include "util.h" @@ -64,6 +57,19 @@ # include #endif +// Multi-OS support for determining if terminal is tty +#if defined(MSDOS) || defined(OS2) || defined(WIN32) || defined(_WIN32) || \ + defined(__CYGWIN__) +#include /* _isatty */ +#define IS_CONSOLE(stdStream) _isatty(_fileno(stdStream)) +#elif defined(_POSIX_C_SOURCE) || defined(_XOPEN_SOURCE) || defined(_POSIX_SOURCE) || (defined(__APPLE__) && defined(__MACH__)) || \ + defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) /* https://sourceforge.net/p/predef/wiki/OperatingSystems/ */ +#include /* isatty */ +#define IS_CONSOLE(stdStream) isatty(fileno(stdStream)) +#else +#define IS_CONSOLE(stdStream) 0 +#endif + /*-************************************* * Constants @@ -2032,14 +2038,11 @@ static int FIO_listFile(fileInfo_t* total, const char* inFileName, int displayLe } int FIO_listMultipleFiles(unsigned numFiles, const char** filenameTable, int displayLevel){ - // isatty comes from the header unistd.h - // windows has no equilivant - #if !defined(_MSC_VER) - if (!isatty(0)) { + + if (!IS_CONSOLE(stdin)) { DISPLAYOUT("zstd: --list does not support reading from standard input\n"); return 1; } - #endif if (numFiles == 0) { DISPLAYOUT("No files given\n"); From b024e1e1f4813f8ee01b888337ff0cb87112c76e Mon Sep 17 00:00:00 2001 From: Topher Lubaway Date: Tue, 12 Jun 2018 10:16:27 -0700 Subject: [PATCH 5/8] Keep windows specific headers Accidentially deleted this existing windows only header --- programs/fileio.c | 1 + 1 file changed, 1 insertion(+) diff --git a/programs/fileio.c b/programs/fileio.c index d33c7f42..7ec4114b 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -61,6 +61,7 @@ #if defined(MSDOS) || defined(OS2) || defined(WIN32) || defined(_WIN32) || \ defined(__CYGWIN__) #include /* _isatty */ +#include #define IS_CONSOLE(stdStream) _isatty(_fileno(stdStream)) #elif defined(_POSIX_C_SOURCE) || defined(_XOPEN_SOURCE) || defined(_POSIX_SOURCE) || (defined(__APPLE__) && defined(__MACH__)) || \ defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) /* https://sourceforge.net/p/predef/wiki/OperatingSystems/ */ From ec24f98ccab4057af684d70c18191bccf1ccbcc0 Mon Sep 17 00:00:00 2001 From: Topher Lubaway Date: Wed, 13 Jun 2018 13:39:23 -0700 Subject: [PATCH 6/8] Removes duplicate IS_CONSOLE from PR I misunderstood that this function was included already --- programs/fileio.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 7ec4114b..90d04d7f 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -57,21 +57,11 @@ # include #endif -// Multi-OS support for determining if terminal is tty -#if defined(MSDOS) || defined(OS2) || defined(WIN32) || defined(_WIN32) || \ - defined(__CYGWIN__) -#include /* _isatty */ -#include -#define IS_CONSOLE(stdStream) _isatty(_fileno(stdStream)) -#elif defined(_POSIX_C_SOURCE) || defined(_XOPEN_SOURCE) || defined(_POSIX_SOURCE) || (defined(__APPLE__) && defined(__MACH__)) || \ - defined(__DragonFly__) || defined(__FreeBSD__) || defined(__NetBSD__) || defined(__OpenBSD__) /* https://sourceforge.net/p/predef/wiki/OperatingSystems/ */ -#include /* isatty */ -#define IS_CONSOLE(stdStream) isatty(fileno(stdStream)) -#else -#define IS_CONSOLE(stdStream) 0 +#if defined (_MSC_VER) +# include +# include #endif - /*-************************************* * Constants ***************************************/ From 6bca3fb4bf2780cee043f94fd8cc0cb22b6b6bd6 Mon Sep 17 00:00:00 2001 From: Topher Lubaway Date: Wed, 13 Jun 2018 14:32:59 -0700 Subject: [PATCH 7/8] Reduce noise in diff putting the code block back on the exact line it came from --- programs/fileio.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/programs/fileio.c b/programs/fileio.c index 90d04d7f..03e8269f 100644 --- a/programs/fileio.c +++ b/programs/fileio.c @@ -31,6 +31,11 @@ #include /* strcmp, strlen */ #include /* errno */ +#if defined (_MSC_VER) +# include +# include +#endif + #include "mem.h" #include "fileio.h" #include "util.h" @@ -57,10 +62,6 @@ # include #endif -#if defined (_MSC_VER) -# include -# include -#endif /*-************************************* * Constants From 5bac1db28f383d0d6e05296f157f7dcfdf881452 Mon Sep 17 00:00:00 2001 From: Topher Lubaway Date: Tue, 19 Jun 2018 09:56:37 -0700 Subject: [PATCH 8/8] Tests to verify piped input to `--list` exits 1 I'm following the pattern that i saw in the rest of the test file please tell me if i am using the wrong conventions --- tests/playTests.sh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/playTests.sh b/tests/playTests.sh index 48001c2e..3b872d28 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -729,6 +729,9 @@ $ECHO "\n===> zstd --list/-l error detection tests " ! $ZSTD -lv tmp1* ! $ZSTD --list -v tmp2 tmp12.zst +$ECHO "\n===> zstd --list/-l exits 1 when stdin is piped in" +! echo "piped STDIN" | $ZSTD --list + $ECHO "\n===> zstd --list/-l test with null files " ./datagen -g0 > tmp5 $ZSTD tmp5