You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2018/11/25 17:30:17 UTC

svn commit: r1847422 - /subversion/trunk/subversion/svn/list-cmd.c

Author: brane
Date: Sun Nov 25 17:30:17 2018
New Revision: 1847422

URL: http://svn.apache.org/viewvc?rev=1847422&view=rev
Log:
Follow up to r1847384, changing the 'svn ls -vH' output to:

  - fix a bug where the size is between 1000 and 1023 units; and,
  - use the locale-specific decimal separator.

* subversion/svn/list-cmd.c: Include <assert.h> for assert(),
   <math.h> for fabs() and <stdio.h> for sprintf().
  (get_human_readable_size):
   - Update the part of the docstring about negative sizes.
   - Scale to the next higher magnitude when the adjusted size is
     between 1000 and 1023 units.
   - Use standard sprintf() to format the displayed number, because
     APR's formatter does not use locale information.

Modified:
    subversion/trunk/subversion/svn/list-cmd.c

Modified: subversion/trunk/subversion/svn/list-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/list-cmd.c?rev=1847422&r1=1847421&r2=1847422&view=diff
==============================================================================
--- subversion/trunk/subversion/svn/list-cmd.c (original)
+++ subversion/trunk/subversion/svn/list-cmd.c Sun Nov 25 17:30:17 2018
@@ -21,6 +21,10 @@
  * ====================================================================
  */
 
+#include <assert.h>
+#include <math.h>
+#include <stdio.h>
+
 #include "svn_cmdline.h"
 #include "svn_client.h"
 #include "svn_error.h"
@@ -71,7 +75,8 @@ static const apr_uint32_t print_dirent_f
     SVN_DIRENT_CREATED_REV | SVN_DIRENT_LAST_AUTHOR);
 
 /* Converts a file size to human-readable form with base-2 unit suffix.
-   File sizes are never negative, so we don't handle that case. */
+   File sizes are never negative, so we don't handle that case other than
+   making sure that the scale adjustment will work. */
 static const char *
 get_human_readable_size(svn_filesize_t size, apr_pool_t *scratch_pool)
 {
@@ -90,13 +95,14 @@ get_human_readable_size(svn_filesize_t s
       {APR_INT64_C(0x0003FFFFFFFFFFFF), 'E'}, /* exbi */
       {APR_INT64_C(0x0FFFFFFFFFFFFFFF), 'P'}  /* pibi */
     };
+  static const apr_size_t order_size = sizeof(order) / sizeof(order[0]);
 
   const svn_filesize_t abs_size = ((size < 0) ? -size : size);
   double human_readable_size;
 
   /* Find the size mask for the (absolute) file size. It would be sexy to
      do a binary search here, but with only 7 elements in the array ... */
-  apr_size_t index = sizeof(order) / sizeof(order[0]);
+  apr_size_t index = order_size;
   while (index > 0)
     {
       --index;
@@ -111,15 +117,41 @@ get_human_readable_size(svn_filesize_t s
      shift by (index * 10) bits. But we split it into an integer and a
      floating-point division, so that we don't overflow the mantissa at
      very large file sizes. */
+  ;
+  if ((abs_size >> 10 * index) > 999)
+    {
+      /* This assertion should never fail, because we only have 4 binary
+         digits in the petabyte range and so the number of petabytes can't
+         be large enough to enter this conditional block. */
+      assert(index < order_size - 1);
+      ++index;
+    }
   human_readable_size = (index == 0 ? (double)size
                          : (size >> 3 * index) / 128.0 / index);
 
-  /* When the absolute adjusted size is < 10, show tenths of a unit, too.
-     NOTE: This *should* display the locale-specific decimal separator,
-           but apparently APR isn't too hot on localisation. */
-  return apr_psprintf(scratch_pool, "%.*f%c",
-                      (abs_size >> 10 * index < 10) ? 1 : 0,
-                      human_readable_size, order[index].suffix);
+  /* NOTE: We want to display a locale-specific decimal sepratator, but
+           APR's formatter completely ignores the locale. So we use the
+           good, old, standard, *dangerous* sprintf() to format the size.
+
+           But, on the brigt side, we've just made sure that the number has
+           no more than 3 non-fractional digits. So the call to sprintf()
+           here should be safe. */
+  {
+    /*   3 digits (or 2 digits and 1 decimal separator)
+       + 1 unit suffix
+       + 1 negative sign (which should not appear under normal circumstances)
+       + 1 nul terminator
+       ---
+       = 6 characters of space needed in the buffer. */
+    char buffer[8];
+
+    /* When the adjusted size has only one significant digit left of the
+       decimal point, show tenths of a unit, too. */
+    sprintf(buffer, "%.*f%c",
+            fabs(human_readable_size) < 10.0 ? 1 : 0,
+            human_readable_size, order[index].suffix);
+    return apr_pstrdup(scratch_pool, buffer);
+  }
 }
 
 /* This implements the svn_client_list_func2_t API, printing a single