lib/repo: Minor fixes around min-free-space

Summary:
* Remove a useless if condition in prepare_transaction()
* Fix glnx_throw error propagation
* Integer overflow check while parsing min-free-space-size config
* Documentation fixes

Closes: #1663
Approved by: jlebon

man/ostree.repo-config.xml

@@ -126,10 +126,10 @@ Boston, MA 02111-1307, USA.
 
      <varlistentry>
         <term><varname>min-free-space-size</varname></term>
-        <listitem><para>Value (in MB, GB or TB) that specifies a minimum space (in blocks)
+        <listitem><para>Value (in MB, GB or TB) that specifies a minimum space in the
-        in the underlying filesystem to keep free. Also, note that min-free-space-percent
+        underlying filesystem to keep free. Also, note that min-free-space-percent
         and min-free-space-size are mutually exclusive. Examples of acceptable values:
-        500MB, 1GB etc.
+        500MB, 1GB etc. The default value is 0MB, which disables this check.
         </para></listitem>
       </varlistentry>
 

src/libostree/ostree-repo-commit.c

@@ -1632,34 +1632,32 @@ ostree_repo_prepare_transaction (OstreeRepo     *self,
 
   self->in_transaction = TRUE;
   self->cleanup_stagedir = FALSE;
-  if (self->min_free_space_percent >= 0 || self->min_free_space_mb >= 0)
-    {
-      struct statvfs stvfsbuf;
-      if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0)
-        return glnx_throw_errno_prefix (error, "fstatvfs");
 
-      g_mutex_lock (&self->txn_lock);
+  struct statvfs stvfsbuf;
-      self->txn.blocksize = stvfsbuf.f_bsize;
+  if (TEMP_FAILURE_RETRY (fstatvfs (self->repo_dir_fd, &stvfsbuf)) < 0)
-      guint64 reserved_blocks = min_free_space_calculate_reserved_blocks (self, &stvfsbuf);
+    return glnx_throw_errno_prefix (error, "fstatvfs");
-      /* Use the appropriate free block count if we're unprivileged */
+
-      guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree);
+  g_mutex_lock (&self->txn_lock);
-      if (bfree > reserved_blocks)
+  self->txn.blocksize = stvfsbuf.f_bsize;
-        self->txn.max_blocks = bfree - reserved_blocks;
+  guint64 reserved_blocks = min_free_space_calculate_reserved_blocks (self, &stvfsbuf);
-      else
+  /* Use the appropriate free block count if we're unprivileged */
-        {
+  guint64 bfree = (getuid () != 0 ? stvfsbuf.f_bavail : stvfsbuf.f_bfree);
-          guint64 bytes_required = bfree * self->txn.blocksize;
+  if (bfree > reserved_blocks)
-          self->cleanup_stagedir = TRUE;
+    self->txn.max_blocks = bfree - reserved_blocks;
-          g_mutex_unlock (&self->txn_lock);
+  else
-          g_autofree char *formatted_free = g_format_size (bytes_required);
+    {
-          if (self->min_free_space_percent > 0)
+      guint64 bytes_required = bfree * self->txn.blocksize;
-            return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available",
+      self->cleanup_stagedir = TRUE;
-                               self->min_free_space_percent, formatted_free);
-          else
-            return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB would be exceeded, %s available",
-                               self->min_free_space_mb, formatted_free);
-        }
       g_mutex_unlock (&self->txn_lock);
+      g_autofree char *formatted_free = g_format_size (bytes_required);
+      if (self->min_free_space_percent > 0)
+        return glnx_throw (error, "min-free-space-percent '%u%%' would be exceeded, %s available",
+                           self->min_free_space_percent, formatted_free);
+      else
+        return glnx_throw (error, "min-free-space-size %" G_GUINT64_FORMAT "MB would be exceeded, %s available",
+                           self->min_free_space_mb, formatted_free);
     }
+  g_mutex_unlock (&self->txn_lock);
 
   gboolean ret_transaction_resume = FALSE;
   if (!_ostree_repo_allocate_tmpdir (self->tmp_dir_fd,

src/libostree/ostree-repo.c

@@ -2670,7 +2670,7 @@ min_free_space_size_validate_and_convert (OstreeRepo    *self,
 
   g_autoptr(GMatchInfo) match = NULL;
   if (!g_regex_match (regex, min_free_space_size_str, 0, &match))
-    return glnx_prefix_error (error, "Failed to parse min-free-space-size parameter: '%s'", min_free_space_size_str);
+    return glnx_throw (error, "Failed to match '^[0-9]+[GMT]B$'");
 
   g_autofree char *size_str = g_match_info_fetch (match, 1);
   g_autofree char *unit = g_match_info_fetch (match, 2);
@@ -2691,7 +2691,11 @@ min_free_space_size_validate_and_convert (OstreeRepo    *self,
         g_assert_not_reached ();
     }
 
-  self->min_free_space_mb = g_ascii_strtoull (size_str, NULL, 10) << shifts;
+  guint64 min_free_space = g_ascii_strtoull (size_str, NULL, 10);
+  if (shifts > 0 && g_bit_nth_lsf (min_free_space, 63 - shifts) != -1)
+    return glnx_throw (error, "Integer overflow detected");
+
+  self->min_free_space_mb = min_free_space << shifts;
 
   return TRUE;
 }
@@ -2829,7 +2833,7 @@ reload_core_config (OstreeRepo          *self,
 
         /* Validate the string and convert the size to MBs */
         if (!min_free_space_size_validate_and_convert (self, min_free_space_size_str, error))
-          return glnx_throw (error, "Invalid min-free-space-size '%s'", min_free_space_size_str);
+          return glnx_prefix_error (error, "Invalid min-free-space-size '%s'", min_free_space_size_str);
       }
     else
       {