[libblockdev 1/3] Omit always true condition and document some invariants.

mulhern amulhern at redhat.com
Wed Sep 3 15:41:31 UTC 2014


i < NUM_PREFIXES is always true in the for-statement.

It should be omitted not for performance reasons but because it is likely
to cause the misguided programmer to pursue the following train of thought:

That i < NUM_PREFIXES condition must be there for a reason. But the only
reason for it to be there as part of the conjunction is because there is
some condition under which it is false. In that case, there is some
condition under which i is equal to 7 when the loop is exited. In that case,
it is possible for i to be 7 when size_prefixes[i] is evaluated (unless
10 is always greater than value when i is 7 which seems highly unlikely).
But size_prefixes only has 7 elements. That means that the code
can access outside the bounds of the array, which is a bug. Oh no, what
do we do now?

The invariants are significant ones since it is they that ensure that
an out-of-bounds access can not possibly occur.

Omitting the deceptive condition and documenting the significant invariants
should prevent any subsequent programmers from even temporarily following
the incorrect train of thought described above.

Signed-off-by: mulhern <amulhern at redhat.com>
---
 src/utils/sizes.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/utils/sizes.c b/src/utils/sizes.c
index d4eb622..91e2713 100644
--- a/src/utils/sizes.c
+++ b/src/utils/sizes.c
@@ -37,16 +37,28 @@ gchar* bd_utils_size_human_readable (guint64 size) {
     gdouble value = (gdouble) size;
     gdouble prev_value = (gdouble) value;
 
-    for (i=0; i < NUM_PREFIXES && INT_LT_FLOAT(1024, value); i++) {
+    for (i=0; INT_LT_FLOAT(1024, value); i++) {
         prev_value = value;
         value = value / 1024.0;
     }
 
+    /*
+     * Since the type of size is guint64, its maximum value is 2^64 - 1.
+     * ((2^64 - 1) / 1024^6) == 15 which is less than 1024. Consequently,
+     * the loop above must terminate after at most six iterations and the
+     * maximum value of i at this point is 6.
+     */
+
     if (INT_GT_FLOAT(10, value)) {
         value = prev_value;
         i = i - 1;
     }
 
+    /*
+     * At this point the maximum value of i is 6 so any size_prefixes accesses
+     * below are guaranteed to remain within bounds.
+     */
+
     if (INT_EQ_FLOAT (value, (guint64) value))
         return g_strdup_printf ("%"G_GUINT64_FORMAT" %sB", (guint64) value, size_prefixes[i]);
     else
-- 
1.9.3



More information about the anaconda-patches mailing list