grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] misc: Make grub_min() and grub_max() more resilient.


From: Peter Jones
Subject: [PATCH] misc: Make grub_min() and grub_max() more resilient.
Date: Thu, 24 Mar 2022 16:06:38 -0400

grub_min(a,b) and grub_max(a,b) use a relatively naive implementation
which leads to several problems:
- they evaluate their parameters more than once
- the naive way to address this, to declare temporary variables in a
  statement-expression, isn't resilient against nested uses, because
  MIN(a,MIN(b,c)) results in the temporary variables being declared in
  two nested scopes, which may result in a build warning depending on
  your build options.

This patch changes our implementation to use a statement-expression
inside a helper macro, and creates the symbols for the temporary
variables with __COUNTER__ (A GNU C cpp extension) and token pasting to
create uniquely named internal variables.

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 grub-core/fs/reiserfs.c            | 28 +++++++++-------------------
 grub-core/loader/multiboot_elfxx.c |  4 +---
 include/grub/misc.h                | 25 +++++++++++++++++++++++--
 3 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/grub-core/fs/reiserfs.c b/grub-core/fs/reiserfs.c
index 36b26ac98a0..42818c37622 100644
--- a/grub-core/fs/reiserfs.c
+++ b/grub-core/fs/reiserfs.c
@@ -42,16 +42,6 @@
 
 GRUB_MOD_LICENSE ("GPLv3+");
 
-#define MIN(a, b) \
-  ({ typeof (a) _a = (a); \
-     typeof (b) _b = (b); \
-     _a < _b ? _a : _b; })
-
-#define MAX(a, b) \
-  ({ typeof (a) _a = (a); \
-     typeof (b) _b = (b); \
-     _a > _b ? _a : _b; })
-
 #define REISERFS_SUPER_BLOCK_OFFSET 0x10000
 #define REISERFS_MAGIC_LEN 12
 #define REISERFS_MAGIC_STRING "ReIsEr"
@@ -1076,7 +1066,7 @@ grub_reiserfs_read_real (struct grub_fshelp_node *node,
   grub_reiserfs_set_key_type (&key, GRUB_REISERFS_ANY, 2);
   initial_position = off;
   current_position = 0;
-  final_position = MIN (len + initial_position, node->size);
+  final_position = grub_min (len + initial_position, node->size);
   grub_dprintf ("reiserfs",
                "Reading from %lld to %lld (%lld instead of requested %ld)\n",
                (unsigned long long) initial_position,
@@ -1115,8 +1105,8 @@ grub_reiserfs_read_real (struct grub_fshelp_node *node,
           grub_dprintf ("reiserfs_blocktype", "D: %u\n", (unsigned) block);
           if (initial_position < current_position + item_size)
             {
-              offset = MAX ((signed) (initial_position - current_position), 0);
-              length = (MIN (item_size, final_position - current_position)
+              offset = grub_max ((signed) (initial_position - 
current_position), 0);
+              length = (grub_min (item_size, final_position - current_position)
                         - offset);
               grub_dprintf ("reiserfs",
                             "Reading direct block %u from %u to %u...\n",
@@ -1161,9 +1151,9 @@ grub_reiserfs_read_real (struct grub_fshelp_node *node,
               grub_dprintf ("reiserfs_blocktype", "I: %u\n", (unsigned) block);
               if (current_position + block_size >= initial_position)
                 {
-                  offset = MAX ((signed) (initial_position - current_position),
-                                0);
-                  length = (MIN (block_size, final_position - current_position)
+                  offset = grub_max ((signed) (initial_position - 
current_position),
+                                    0);
+                  length = (grub_min (block_size, final_position - 
current_position)
                             - offset);
                   grub_dprintf ("reiserfs",
                                 "Reading indirect block %u from %u to %u...\n",
@@ -1205,7 +1195,7 @@ grub_reiserfs_read_real (struct grub_fshelp_node *node,
   switch (found.type)
     {
       case GRUB_REISERFS_DIRECT:
-        read_length = MIN (len, item_size - file->offset);
+        read_length = grub_min (len, item_size - file->offset);
         grub_disk_read (found.data->disk,
                         (found.block_number * block_size) / 
GRUB_DISK_SECTOR_SIZE,
                         grub_le_to_cpu16 (found.header.item_location) + 
file->offset,
@@ -1224,12 +1214,12 @@ grub_reiserfs_read_real (struct grub_fshelp_node *node,
                         item_size, (char *) indirect_block_ptr);
         if (grub_errno)
           goto fail;
-        len = MIN (len, file->size - file->offset);
+        len = grub_min (len, file->size - file->offset);
         for (indirect_block = file->offset / block_size;
              indirect_block < indirect_block_count && read_length < len;
              indirect_block++)
           {
-            read = MIN (block_size, len - read_length);
+            read = grub_min (block_size, len - read_length);
             grub_disk_read (found.data->disk,
                             (grub_le_to_cpu32 
(indirect_block_ptr[indirect_block]) * block_size) / GRUB_DISK_SECTOR_SIZE,
                             file->offset % block_size, read,
diff --git a/grub-core/loader/multiboot_elfxx.c 
b/grub-core/loader/multiboot_elfxx.c
index 6801f0ddf2b..8744be14a46 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -35,9 +35,7 @@
 #endif
 
 #include <grub/i386/relocator.h>
-
-#define CONCAT(a,b)    CONCAT_(a, b)
-#define CONCAT_(a,b)   a ## b
+#include <grub/misc.h>
 
 #pragma GCC diagnostic ignored "-Wcast-align"
 
diff --git a/include/grub/misc.h b/include/grub/misc.h
index 7d2b5519690..83b31799722 100644
--- a/include/grub/misc.h
+++ b/include/grub/misc.h
@@ -35,6 +35,14 @@
 #define ARRAY_SIZE(array) (sizeof (array) / sizeof (array[0]))
 #define COMPILE_TIME_ASSERT(cond) switch (0) { case 1: case !(cond): ; }
 
+#ifndef CONCAT_
+#define CONCAT_(a, b) a ## b
+#endif
+
+#ifndef CONCAT
+#define CONCAT(a, b) CONCAT_(a, b)
+#endif
+
 #define grub_dprintf(condition, ...) grub_real_dprintf(GRUB_FILE, __LINE__, 
condition, __VA_ARGS__)
 
 void *EXPORT_FUNC(grub_memmove) (void *dest, const void *src, grub_size_t n);
@@ -495,8 +503,21 @@ void EXPORT_FUNC(grub_real_boot_time) (const char *file,
 #define grub_boot_time(...)
 #endif
 
-#define grub_max(a, b) (((a) > (b)) ? (a) : (b))
-#define grub_min(a, b) (((a) < (b)) ? (a) : (b))
+#define _grub_min(a, b, _a, _b)                                                
      \
+  ({ typeof (a) _a = (a);                                                    \
+     typeof (b) _b = (b);                                                    \
+     _a < _b ? _a : _b; })
+#define grub_min(a, b) _grub_min(a, b,                                       \
+                                CONCAT(_a_,__COUNTER__),                     \
+                                CONCAT(_b_,__COUNTER__))
+
+#define _grub_max(a, b, _a, _b)                                                
      \
+  ({ typeof (a) _a = (a);                                                    \
+     typeof (b) _b = (b);                                                    \
+     _a > _b ? _a : _b; })
+#define grub_max(a, b) _grub_max(a, b,                                       \
+                                CONCAT(_a_,__COUNTER__),                     \
+                                CONCAT(_b_,__COUNTER__))
 
 #define grub_log2ull(n) (GRUB_TYPE_BITS (grub_uint64_t) - __builtin_clzll (n) 
- 1)
 
-- 
2.34.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]