Fixes to progress message handling in xz:
authorLasse Collin <lasse.collin@tukaani.org>
Sun, 22 Feb 2009 16:52:49 +0000 (18:52 +0200)
committerLasse Collin <lasse.collin@tukaani.org>
Sun, 22 Feb 2009 16:52:49 +0000 (18:52 +0200)
  - Don't use Windows-specific code on Windows. The old code
    required at least Windows 2000. Now it should work on
    Windows 98 and later, and maybe on Windows 95 too.

  - Use less precision when showing estimated remaining time.

  - Fix some small design issues.

src/xz/message.c
src/xz/message.h
src/xz/process.c

index a89c5a4..eba7205 100644 (file)
 #      include <sys/time.h>
 #endif
 
-#ifdef _WIN32
-#      ifndef _WIN32_WINNT
-#              define _WIN32_WINNT 0x0500
-#      endif
-#      include <windows.h>
-#endif
-
 #include <stdarg.h>
 
 
@@ -51,100 +44,80 @@ static const char *filename;
 /// True once the a filename has been printed to stderr as part of progress
 /// message. If automatic progress updating isn't enabled, this becomes true
 /// after the first progress message has been printed due to user sending
-/// SIGALRM. Once this variable is true,  we will print an empty line before
-/// the next filename to make the output more readable.
+/// SIGINFO, SIGUSR1, or SIGALRM. Once this variable is true, we will print
+/// an empty line before the next filename to make the output more readable.
 static bool first_filename_printed = false;
 
 /// This is set to true when we have printed the current filename to stderr
 /// as part of a progress message. This variable is useful only if not
-/// updating progress automatically: if user sends many SIGALRM signals,
-/// we won't print the name of the same file multiple times.
+/// updating progress automatically: if user sends many SIGINFO, SIGUSR1, or
+/// SIGALRM signals, we won't print the name of the same file multiple times.
 static bool current_filename_printed = false;
 
-/// True if we should print progress indicator and update it automatically.
+/// True if we should print progress indicator and update it automatically
+/// if also verbose >= V_VERBOSE.
 static bool progress_automatic;
 
+/// True if message_progress_start() has been called but
+/// message_progress_end() hasn't been called yet.
+static bool progress_started = false;
+
 /// This is true when a progress message was printed and the cursor is still
 /// on the same line with the progress message. In that case, a newline has
 /// to be printed before any error messages.
 static bool progress_active = false;
 
+/// Pointer to lzma_stream used to do the encoding or decoding.
+static lzma_stream *progress_strm;
+
 /// Expected size of the input stream is needed to show completion percentage
 /// and estimate remaining time.
 static uint64_t expected_in_size;
 
 /// Time when we started processing the file
-static double start_time;
+static uint64_t start_time;
+
+
+// Use alarm() and SIGALRM when they are supported. This has two minor
+// advantages over the alternative of polling gettimeofday():
+//  - It is possible for the user to send SIGINFO, SIGUSR1, or SIGALRM to
+//    get intermediate progress information even when --verbose wasn't used
+//    or stderr is not a terminal.
+//  - alarm() + SIGALRM seems to have slightly less overhead than polling
+//    gettimeofday().
+#ifdef SIGALRM
 
 /// The signal handler for SIGALRM sets this to true. It is set back to false
 /// once the progress message has been updated.
 static volatile sig_atomic_t progress_needs_updating = false;
 
-
-#ifdef _WIN32
-
-static HANDLE timer_queue = NULL;
-static HANDLE timer_timer = NULL;
-
-
-static void CALLBACK
-timer_callback(PVOID dummy1 lzma_attribute((unused)),
-               BOOLEAN dummy2 lzma_attribute((unused)))
-{
-       progress_needs_updating = true;
-       return;
-}
-
-
-/// Emulate alarm() on Windows.
+/// Signal handler for SIGALRM
 static void
-my_alarm(unsigned int seconds)
+progress_signal_handler(int sig lzma_attribute((unused)))
 {
-       // Just in case creating the queue has failed.
-       if (timer_queue == NULL)
-               return;
-
-       // If an old timer_timer exists, get rid of it first.
-       if (timer_timer != NULL) {
-               (void)DeleteTimerQueueTimer(timer_queue, timer_timer, NULL);
-               timer_timer = NULL;
-       }
-
-       // If it fails, tough luck. It's not that important.
-       (void)CreateTimerQueueTimer(&timer_timer, timer_queue, &timer_callback,
-                       NULL, 1000U * seconds, 0,
-                       WT_EXECUTEINTIMERTHREAD | WT_EXECUTEONLYONCE);
-
+       progress_needs_updating = true;
        return;
 }
 
 #else
 
-#define my_alarm alarm
+/// This is true when progress message printing is wanted. Using the same
+/// variable name as above to avoid some ifdefs.
+static bool progress_needs_updating = false;
 
-/// Signal handler for SIGALRM
-static void
-progress_signal_handler(int sig lzma_attribute((unused)))
-{
-       progress_needs_updating = true;
-       return;
-}
+/// Elapsed time when the next progress message update should be done.
+static uint64_t progress_next_update;
 
 #endif
 
-/// Get the current time as double
-static double
+
+/// Get the current time as microseconds since epoch
+static uint64_t
 my_time(void)
 {
        struct timeval tv;
-
-       // This really shouldn't fail. I'm not sure what to return if it
-       // still fails. It doesn't look so useful to check the return value
-       // everywhere. FIXME?
-       if (gettimeofday(&tv, NULL))
-               return -1.0;
-
-       return (double)(tv.tv_sec) + (double)(tv.tv_usec) / 1.0e6;
+       gettimeofday(&tv, NULL);
+       return (uint64_t)(tv.tv_sec) * UINT64_C(1000000) + tv.tv_usec;
 }
 
 
@@ -204,20 +177,36 @@ message_init(const char *given_argv0)
        }
 */
 
-#ifdef _WIN32
-       timer_queue = CreateTimerQueue();
-#else
+#ifdef SIGALRM
+       // At least DJGPP lacks SA_RESTART. It's not essential for us (the
+       // rest of the code can handle interrupted system calls), so just
+       // define it zero.
 #      ifndef SA_RESTART
 #              define SA_RESTART 0
 #      endif
-       // Establish the signal handler for SIGALRM. Since this signal
-       // doesn't require any quick action, we set SA_RESTART.
+       // Establish the signal handlers which set a flag to tell us that
+       // progress info should be updated. Since these signals don't
+       // require any quick action, we set SA_RESTART.
+       static const int sigs[] = {
+#ifdef SIGALRM
+               SIGALRM,
+#endif
+#ifdef SIGINFO
+               SIGINFO,
+#endif
+#ifdef SIGUSR1
+               SIGUSR1,
+#endif
+       };
+
        struct sigaction sa;
        sigemptyset(&sa.sa_mask);
        sa.sa_flags = SA_RESTART;
        sa.sa_handler = &progress_signal_handler;
-       if (sigaction(SIGALRM, &sa, NULL))
-               message_signal_handler();
+
+       for (size_t i = 0; i < ARRAY_SIZE(sigs); ++i)
+               if (sigaction(sigs[i], &sa, NULL))
+                       message_signal_handler();
 #endif
 
        return;
@@ -288,16 +277,25 @@ print_filename(void)
 
 
 extern void
-message_progress_start(const char *src_name, uint64_t in_size)
+message_progress_start(
+               lzma_stream *strm, const char *src_name, uint64_t in_size)
 {
+       // Store the pointer to the lzma_stream used to do the coding.
+       // It is needed to find out the position in the stream.
+       progress_strm = strm;
+
        // Store the processing start time of the file and its expected size.
        // If we aren't printing any statistics, then these are unused. But
-       // since it is possible that the user tells us with SIGALRM to show
+       // since it is possible that the user sends us a signal to show
        // statistics, we need to have these available anyway.
        start_time = my_time();
        filename = src_name;
        expected_in_size = in_size;
 
+       // Indicate that progress info may need to be printed before
+       // printing error messages.
+       progress_started = true;
+
        // Indicate the name of this file hasn't been printed to
        // stderr yet.
        current_filename_printed = false;
@@ -306,19 +304,26 @@ message_progress_start(const char *src_name, uint64_t in_size)
        ++files_pos;
 
        // If progress indicator is wanted, print the filename and possibly
-       // the file count now. As an exception, if there is exactly one file,
-       // do not print the filename at all.
+       // the file count now.
        if (verbosity >= V_VERBOSE && progress_automatic) {
                // Print the filename to stderr if that is appropriate with
                // the current settings.
                print_filename();
 
-               // Start the timer to set progress_needs_updating to true
-               // after about one second. An alternative would to be set
-               // progress_needs_updating to true here immediatelly, but
-               // setting the timer looks better to me, since extremely
-               // early progress info is pretty much useless.
-               my_alarm(1);
+               // Start the timer to display the first progress message
+               // after one second. An alternative would be to show the
+               // first message almost immediatelly, but delaying by one
+               // second looks better to me, since extremely early
+               // progress info is pretty much useless.
+#ifdef SIGALRM
+               // First disable a possibly existing alarm.
+               alarm(0);
+               progress_needs_updating = false;
+               alarm(1);
+#else
+               progress_needs_updating = true;
+               progress_next_update = 1000000;
+#endif
        }
 
        return;
@@ -327,21 +332,31 @@ message_progress_start(const char *src_name, uint64_t in_size)
 
 /// Make the string indicating completion percentage.
 static const char *
-progress_percentage(uint64_t in_pos)
+progress_percentage(uint64_t in_pos, bool final)
 {
-       // If the size of the input file is unknown or the size told us is
-       // clearly wrong since we have processed more data than the alleged
-       // size of the file, show a static string indicating that we have
-       // no idea of the completion percentage.
-       if (expected_in_size == 0 || in_pos > expected_in_size)
-               return "--- %";
+       static char buf[sizeof("100.0 %")];
 
-       static char buf[sizeof("99.9 %")];
+       double percentage;
+
+       if (final) {
+               // Use floating point conversion of snprintf() also for
+               // 100.0 % instead of fixed string, because the decimal
+               // separator isn't a dot in all locales.
+               percentage = 100.0;
+       } else {
+               // If the size of the input file is unknown or the size told us is
+               // clearly wrong since we have processed more data than the alleged
+               // size of the file, show a static string indicating that we have
+               // no idea of the completion percentage.
+               if (expected_in_size == 0 || in_pos > expected_in_size)
+                       return "--- %";
+
+               // Never show 100.0 % before we actually are finished.
+               percentage = (double)(in_pos) / (double)(expected_in_size)
+                               * 99.9;
+       }
 
-       // Never show 100.0 % before we actually are finished (that case is
-       // handled separately in message_progress_end()).
-       snprintf(buf, sizeof(buf), "%.1f %%",
-                       (double)(in_pos) / (double)(expected_in_size) * 99.9);
+       snprintf(buf, sizeof(buf), "%.1f %%", percentage);
 
        return buf;
 }
@@ -350,6 +365,8 @@ progress_percentage(uint64_t in_pos)
 static void
 progress_sizes_helper(char **pos, size_t *left, uint64_t value, bool final)
 {
+       // Allow high precision only for the final message, since it looks
+       // stupid for in-progress information.
        if (final) {
                // At maximum of four digits is allowed for exact byte count.
                if (value < 10000) {
@@ -368,6 +385,7 @@ progress_sizes_helper(char **pos, size_t *left, uint64_t value, bool final)
        // Otherwise we use MiB.
        my_snprintf(pos, left, "%'.1f MiB",
                        (double)(value) / (1024.0 * 1024.0));
+
        return;
 }
 
@@ -412,11 +430,11 @@ progress_sizes(uint64_t compressed_pos, uint64_t uncompressed_pos, bool final)
 
 /// Make the string containing the processing speed of uncompressed data.
 static const char *
-progress_speed(uint64_t uncompressed_pos, double elapsed)
+progress_speed(uint64_t uncompressed_pos, uint64_t elapsed)
 {
        // Don't print the speed immediatelly, since the early values look
        // like somewhat random.
-       if (elapsed < 3.0)
+       if (elapsed < 3000000)
                return "";
 
        static const char unit[][8] = {
@@ -428,17 +446,24 @@ progress_speed(uint64_t uncompressed_pos, double elapsed)
        size_t unit_index = 0;
 
        // Calculate the speed as KiB/s.
-       double speed = (double)(uncompressed_pos) / (elapsed * 1024.0);
+       double speed = (double)(uncompressed_pos)
+                       / ((double)(elapsed) * (1024.0 / 1e6));
 
        // Adjust the unit of the speed if needed.
-       while (speed > 999.9) {
+       while (speed > 999.0) {
                speed /= 1024.0;
                if (++unit_index == ARRAY_SIZE(unit))
                        return ""; // Way too fast ;-)
        }
 
-       static char buf[sizeof("999.9 GiB/s")];
-       snprintf(buf, sizeof(buf), "%.1f %s", speed, unit[unit_index]);
+       // Use decimal point only if the number is small. Examples:
+       //  - 0.1 KiB/s
+       //  - 9.9 KiB/s
+       //  - 99 KiB/s
+       //  - 999 KiB/s
+       static char buf[sizeof("999 GiB/s")];
+       snprintf(buf, sizeof(buf), "%.*f %s",
+                       speed > 9.9 ? 0 : 1, speed, unit[unit_index]);
        return buf;
 }
 
@@ -446,13 +471,15 @@ progress_speed(uint64_t uncompressed_pos, double elapsed)
 /// Make a string indicating elapsed or remaining time. The format is either
 /// M:SS or H:MM:SS depending on if the time is an hour or more.
 static const char *
-progress_time(uint32_t seconds)
+progress_time(uint64_t useconds)
 {
        // 9999 hours = 416 days
        static char buf[sizeof("9999:59:59")];
 
+       uint32_t seconds = useconds / 1000000;
+
        // Don't show anything if the time is zero or ridiculously big.
-       if (seconds == 0 || seconds > ((UINT32_C(9999) * 60) + 59) * 60 + 59)
+       if (seconds == 0 || seconds > ((9999 * 60) + 59) * 60 + 59)
                return "";
 
        uint32_t minutes = seconds / 60;
@@ -476,87 +503,187 @@ progress_time(uint32_t seconds)
 /// Make the string to contain the estimated remaining time, or if the amount
 /// of input isn't known, how much time has elapsed.
 static const char *
-progress_remaining(uint64_t in_pos, double elapsed)
+progress_remaining(uint64_t in_pos, uint64_t elapsed)
 {
-       // If we don't know the size of the input, we indicate the time
-       // spent so far.
-       if (expected_in_size == 0 || in_pos > expected_in_size)
-               return progress_time((uint32_t)(elapsed));
-
-       // If we are at the very beginning of the file or the file is very
-       // small, don't give any estimate to avoid far too wrong estimations.
-       if (in_pos < (UINT64_C(1) << 19) || elapsed < 8.0)
-               return "";
+       // Show the amount of time spent so far when making an estimate of
+       // remaining time wouldn't be reasonable:
+       //  - Input size is unknown.
+       //  - Input has grown bigger since we started (de)compressing.
+       //  - We haven't processed much data yet, so estimate would be
+       //    too inaccurate.
+       //  - Only a few seconds has passed since we started (de)compressing,
+       //    so estimate would be too inaccurate.
+       if (expected_in_size == 0 || in_pos > expected_in_size
+                       || in_pos < (UINT64_C(1) << 19) || elapsed < 8000000)
+               return progress_time(elapsed);
 
        // Calculate the estimate. Don't give an estimate of zero seconds,
        // since it is possible that all the input has been already passed
        // to the library, but there is still quite a bit of output pending.
        uint32_t remaining = (double)(expected_in_size - in_pos)
-                       * elapsed / (double)(in_pos);
-       if (remaining == 0)
+                       * ((double)(elapsed) / 1e6) / (double)(in_pos);
+       if (remaining < 1)
                remaining = 1;
 
-       return progress_time(remaining);
+       static char buf[sizeof("9 h 55 min")];
+
+       // Select appropriate precision for the estimated remaining time.
+       if (remaining <= 10) {
+               // At maximum of 10 seconds remaining.
+               // Show the number of seconds as is.
+               snprintf(buf, sizeof(buf), "%" PRIu32 " s", remaining);
+
+       } else if (remaining <= 50) {
+               // At maximum of 50 seconds remaining.
+               // Round up to the next multiple of five seconds.
+               remaining = (remaining + 4) / 5 * 5;
+               snprintf(buf, sizeof(buf), "%" PRIu32 " s", remaining);
+
+       } else if (remaining <= 590) {
+               // At maximum of 9 minutes and 50 seconds remaining.
+               // Round up to the next multiple of ten seconds.
+               remaining = (remaining + 9) / 10 * 10;
+               snprintf(buf, sizeof(buf), "%" PRIu32 " min %" PRIu32 " s",
+                               remaining / 60, remaining % 60);
+
+       } else if (remaining <= 59 * 60) {
+               // At maximum of 59 minutes remaining.
+               // Round up to the next multiple of a minute.
+               remaining = (remaining + 59) / 60;
+               snprintf(buf, sizeof(buf), "%" PRIu32 " min", remaining);
+
+       } else if (remaining <= 9 * 3600 + 50 * 60) {
+               // At maximum of 9 hours and 50 minutes left.
+               // Round up to the next multiple of ten minutes.
+               remaining = (remaining + 599) / 600 * 10;
+               snprintf(buf, sizeof(buf), "%" PRIu32 " h %" PRIu32 " min",
+                               remaining / 60, remaining % 60);
+
+       } else if (remaining <= 23 * 3600) {
+               // At maximum of 23 hours remaining.
+               // Round up to the next multiple of an hour.
+               remaining = (remaining + 3599) / 3600;
+               snprintf(buf, sizeof(buf), "%" PRIu32 " h", remaining);
+
+       } else if (remaining <= 9 * 24 * 3600 + 23 * 3600) {
+               // At maximum of 9 days and 23 hours remaining.
+               // Round up to the next multiple of an hour.
+               remaining = (remaining + 3599) / 3600;
+               snprintf(buf, sizeof(buf), "%" PRIu32 " d %" PRIu32 " h",
+                               remaining / 24, remaining % 24);
+
+       } else if (remaining <= 999 * 24 * 3600) {
+               // At maximum of 999 days remaining. ;-)
+               // Round up to the next multiple of a day.
+               remaining = (remaining + 24 * 3600 - 1) / (24 * 3600);
+               snprintf(buf, sizeof(buf), "%" PRIu32 " d", remaining);
+
+       } else {
+               // The estimated remaining time is so big that it's better
+               // that we just show the elapsed time.
+               return progress_time(elapsed);
+       }
+
+       return buf;
+}
+
+
+/// Calculate the elapsed time as microseconds.
+static uint64_t
+progress_elapsed(void)
+{
+       return my_time() - start_time;
+}
+
+
+/// Get information about position in the stream. This is currently simple,
+/// but it will become more complicated once we have multithreading support.
+static void
+progress_pos(uint64_t *in_pos,
+               uint64_t *compressed_pos, uint64_t *uncompressed_pos)
+{
+       *in_pos = progress_strm->total_in;
+
+       if (opt_mode == MODE_COMPRESS) {
+               *compressed_pos = progress_strm->total_out;
+               *uncompressed_pos = progress_strm->total_in;
+       } else {
+               *compressed_pos = progress_strm->total_in;
+               *uncompressed_pos = progress_strm->total_out;
+       }
+
+       return;
 }
 
 
 extern void
-message_progress_update(uint64_t in_pos, uint64_t out_pos)
+message_progress_update(void)
 {
-       // If there's nothing to do, return immediatelly.
-       if (!progress_needs_updating || in_pos == 0)
+       if (!progress_needs_updating)
                return;
 
-       // Print the filename if it hasn't been printed yet.
-       print_filename();
-
        // Calculate how long we have been processing this file.
-       const double elapsed = my_time() - start_time;
+       const uint64_t elapsed = progress_elapsed();
+
+#ifndef SIGALRM
+       if (progress_next_update > elapsed)
+               return;
+
+       progress_next_update = elapsed + 1000000;
+#endif
 
-       // Set compressed_pos and uncompressed_pos.
+       // Get our current position in the stream.
+       uint64_t in_pos;
        uint64_t compressed_pos;
        uint64_t uncompressed_pos;
-       if (opt_mode == MODE_COMPRESS) {
-               compressed_pos = out_pos;
-               uncompressed_pos = in_pos;
-       } else {
-               compressed_pos = in_pos;
-               uncompressed_pos = out_pos;
-       }
+       progress_pos(&in_pos, &compressed_pos, &uncompressed_pos);
 
+       // Block signals so that fprintf() doesn't get interrupted.
        signals_block();
 
+       // Print the filename if it hasn't been printed yet.
+       print_filename();
+
        // Print the actual progress message. The idea is that there is at
        // least three spaces between the fields in typical situations, but
        // even in rare situations there is at least one space.
-       fprintf(stderr, "  %7s %43s   %11s %10s\r",
-               progress_percentage(in_pos),
+       fprintf(stderr, "  %7s %43s   %9s   %10s\r",
+               progress_percentage(in_pos, false),
                progress_sizes(compressed_pos, uncompressed_pos, false),
                progress_speed(uncompressed_pos, elapsed),
                progress_remaining(in_pos, elapsed));
 
+#ifdef SIGALRM
        // Updating the progress info was finished. Reset
        // progress_needs_updating to wait for the next SIGALRM.
        //
-       // NOTE: This has to be done before my_alarm() call or with (very) bad
+       // NOTE: This has to be done before alarm(1) or with (very) bad
        // luck we could be setting this to false after the alarm has already
        // been triggered.
        progress_needs_updating = false;
 
-       if (progress_automatic) {
+       if (verbosity >= V_VERBOSE && progress_automatic) {
                // Mark that the progress indicator is active, so if an error
                // occurs, the error message gets printed cleanly.
                progress_active = true;
 
                // Restart the timer so that progress_needs_updating gets
                // set to true after about one second.
-               my_alarm(1);
+               alarm(1);
        } else {
                // The progress message was printed because user had sent us
                // SIGALRM. In this case, each progress message is printed
                // on its own line.
                fputc('\n', stderr);
        }
+#else
+       // When SIGALRM isn't supported and we get here, it's always due to
+       // automatic progress update. We set progress_active here too like
+       // described above.
+       assert(verbosity >= V_VERBOSE);
+       assert(progress_automatic);
+       progress_active = true;
+#endif
 
        signals_unblock();
 
@@ -564,57 +691,58 @@ message_progress_update(uint64_t in_pos, uint64_t out_pos)
 }
 
 
-extern void
-message_progress_end(uint64_t in_pos, uint64_t out_pos, bool success)
+static void
+progress_flush(bool finished)
 {
-       // If we are not in verbose mode, we have nothing to do.
-       if (verbosity < V_VERBOSE || user_abort)
+       if (!progress_started || verbosity < V_VERBOSE)
                return;
 
-       // Cancel a pending alarm, if any.
-       if (progress_automatic) {
-               my_alarm(0);
-               progress_active = false;
-       }
-
-       const double elapsed = my_time() - start_time;
-
+       uint64_t in_pos;
        uint64_t compressed_pos;
        uint64_t uncompressed_pos;
-       if (opt_mode == MODE_COMPRESS) {
-               compressed_pos = out_pos;
-               uncompressed_pos = in_pos;
-       } else {
-               compressed_pos = in_pos;
-               uncompressed_pos = out_pos;
-       }
+       progress_pos(&in_pos, &compressed_pos, &uncompressed_pos);
+
+       // Avoid printing intermediate progress info if some error occurs
+       // in the beginning of the stream. (If something goes wrong later in
+       // the stream, it is sometimes useful to tell the user where the
+       // error approximately occurred, especially if the error occurs
+       // after a time-consuming operation.)
+       if (!finished && !progress_active
+                       && (compressed_pos == 0 || uncompressed_pos == 0))
+               return;
 
-       // If it took less than a second, don't display the time.
-       const char *elapsed_str = progress_time((double)(elapsed));
+       progress_active = false;
+
+       const uint64_t elapsed = progress_elapsed();
+       const char *elapsed_str = progress_time(elapsed);
 
        signals_block();
 
        // When using the auto-updating progress indicator, the final
        // statistics are printed in the same format as the progress
        // indicator itself.
-       if (progress_automatic && in_pos > 0) {
+       if (progress_automatic) {
                // Using floating point conversion for the percentage instead
                // of static "100.0 %" string, because the decimal separator
                // isn't a dot in all locales.
-               fprintf(stderr, "  %5.1f %% %43s   %11s %10s\n",
-                       100.0,
+               fprintf(stderr, "  %7s %43s   %9s   %10s\n",
+                       progress_percentage(in_pos, finished),
                        progress_sizes(compressed_pos, uncompressed_pos, true),
                        progress_speed(uncompressed_pos, elapsed),
                        elapsed_str);
-
-       // When no automatic progress indicator is used, don't print a verbose
-       // message at all if we something went wrong and we couldn't produce
-       // any output. If we did produce output, then it is sometimes useful
-       // to tell that to the user, especially if we detected an error after
-       // a time-consuming operation.
-       } else if (success || out_pos > 0) {
-               // The filename and size information are always printed.
-               fprintf(stderr, "%s: %s", filename, progress_sizes(
+       } else {
+               // The filename is always printed.
+               fprintf(stderr, "%s: ", filename);
+
+               // Percentage is printed only if we didn't finish yet.
+               // FIXME: This may look weird when size of the input
+               // isn't known.
+               if (!finished)
+                       fprintf(stderr, "%s, ",
+                                       progress_percentage(in_pos, false));
+
+               // Size information is always printed.
+               fprintf(stderr, "%s", progress_sizes(
                                compressed_pos, uncompressed_pos, true));
 
                // The speed and elapsed time aren't always shown.
@@ -634,22 +762,23 @@ message_progress_end(uint64_t in_pos, uint64_t out_pos, bool success)
 }
 
 
+extern void
+message_progress_end(bool success)
+{
+       assert(progress_started);
+       progress_flush(success);
+       progress_started = false;
+       return;
+}
+
+
 static void
 vmessage(enum message_verbosity v, const char *fmt, va_list ap)
 {
        if (v <= verbosity) {
                signals_block();
 
-               // If there currently is a progress message on the screen,
-               // print a newline so that the progress message is left
-               // readable. This is good, because it is nice to be able to
-               // see where the error occurred. (The alternative would be
-               // to clear the progress message and replace it with the
-               // error message.)
-               if (progress_active) {
-                       progress_active = false;
-                       fputc('\n', stderr);
-               }
+               progress_flush(false);
 
                fprintf(stderr, "%s: ", argv0);
                vfprintf(stderr, fmt, ap);
index 3d117fe..24f259c 100644 (file)
@@ -111,21 +111,29 @@ extern void message_version(void) lzma_attribute((noreturn));
 extern void message_help(bool long_help) lzma_attribute((noreturn));
 
 
+/// \brief      Start progress info handling
 ///
-extern void message_progress_start(const char *filename, uint64_t in_size);
+/// This must be paired with a call to message_progress_end() before the
+/// given *strm becomes invalid.
+///
+/// \param      strm      Pointer to lzma_stream used for the coding.
+/// \param      filename  Name of the input file. stdin_filename is
+///                       handled specially.
+/// \param      in_size   Size of the input file, or zero if unknown.
+///
+extern void message_progress_start(
+               lzma_stream *strm, const char *filename, uint64_t in_size);
 
 
-///
-extern void message_progress_update(uint64_t in_pos, uint64_t out_pos);
+/// Update the progress info if in verbose mode and enough time has passed
+/// since the previous update. This can be called only when
+/// message_progress_start() has already been used.
+extern void message_progress_update(void);
 
 
 /// \brief      Finishes the progress message if we were in verbose mode
 ///
-/// \param      in_pos      Final input position i.e. how much input there was.
-/// \param      out_pos     Final output position
-/// \param      success     True if the operation was successful. We don't
-///                         print the final progress message if the operation
-///                         wasn't successful.
+/// \param      finished    True if the whole stream was successfully coded
+///                         and output written to the output stream.
 ///
-extern void message_progress_end(
-               uint64_t in_pos, uint64_t out_pos, bool success);
+extern void message_progress_end(bool finished);
index efe363c..fbdfbb3 100644 (file)
@@ -337,10 +337,11 @@ coder_run(file_pair *pair)
        // Initialize the progress indicator.
        const uint64_t in_size = pair->src_st.st_size <= (off_t)(0)
                        ? 0 : (uint64_t)(pair->src_st.st_size);
-       message_progress_start(pair->src_name, in_size);
+       message_progress_start(&strm, pair->src_name, in_size);
 
        lzma_action action = LZMA_RUN;
        lzma_ret ret;
+       bool success = false; // Assume that something goes wrong.
 
        strm.avail_in = 0;
        strm.next_out = out_buf;
@@ -370,7 +371,7 @@ coder_run(file_pair *pair)
                if (strm.avail_out == 0) {
                        if (opt_mode != MODE_TEST && io_write(pair, out_buf,
                                        IO_BUFFER_SIZE - strm.avail_out))
-                               return false;
+                               break;
 
                        strm.next_out = out_buf;
                        strm.avail_out = IO_BUFFER_SIZE;
@@ -383,18 +384,6 @@ coder_run(file_pair *pair)
                                        && ret != LZMA_UNSUPPORTED_CHECK;
 
                        if (stop) {
-                               // First print the final progress info.
-                               // This way the user sees more accurately
-                               // where the error occurred. Note that we
-                               // print this *before* the possible error
-                               // message.
-                               //
-                               // FIXME: What if something goes wrong
-                               // after this?
-                               message_progress_end(strm.total_in,
-                                               strm.total_out,
-                                               ret == LZMA_STREAM_END);
-
                                // Write the remaining bytes even if something
                                // went wrong, because that way the user gets
                                // as much data as possible, which can be good
@@ -403,21 +392,32 @@ coder_run(file_pair *pair)
                                if (opt_mode != MODE_TEST && io_write(pair,
                                                out_buf, IO_BUFFER_SIZE
                                                        - strm.avail_out))
-                                       return false;
+                                       break;
                        }
 
                        if (ret == LZMA_STREAM_END) {
                                // Check that there is no trailing garbage.
                                // This is needed for LZMA_Alone and raw
                                // streams.
-                               if (strm.avail_in == 0 && (pair->src_eof
-                                               || io_read(pair, in_buf, 1)
-                                                       == 0)) {
-                                       assert(pair->src_eof);
-                                       return true;
+                               if (strm.avail_in == 0 && !pair->src_eof) {
+                                       // Try reading one more byte.
+                                       // Hopefully we don't get any more
+                                       // input, and thus pair->src_eof
+                                       // becomes true.
+                                       strm.avail_in = io_read(
+                                                       pair, in_buf, 1);
+                                       if (strm.avail_in == SIZE_MAX)
+                                               break;
+
+                                       assert(strm.avail_in == 0
+                                                       || strm.avail_in == 1);
                                }
 
-                               // FIXME: What about io_read() failing?
+                               if (strm.avail_in == 0) {
+                                       assert(pair->src_eof);
+                                       success = true;
+                                       break;
+                               }
 
                                // We hadn't reached the end of the file.
                                ret = LZMA_DATA_ERROR;
@@ -461,15 +461,16 @@ coder_run(file_pair *pair)
                        }
 
                        if (stop)
-                               return false;
+                               break;
                }
 
-               // Show progress information if --verbose was specified and
-               // stderr is a terminal.
-               message_progress_update(strm.total_in, strm.total_out);
+               // Show progress information under certain conditions.
+               message_progress_update();
        }
 
-       return false;
+       message_progress_end(success);
+
+       return success;
 }