Hi,

On 2024/5/18 15:52, Zhang Boyang wrote:
Hi,

On 2024/5/17 20:56, Gao Xiang wrote:
The following EROFS patch will use this helper to handle overflow
ALIGN_UP() cases.

Signed-off-by: Gao Xiang <hsiang...@linux.alibaba.com>
---
  include/grub/safemath.h | 16 ++++++++++++++++
  1 file changed, 16 insertions(+)

diff --git a/include/grub/safemath.h b/include/grub/safemath.h
index fbd9b5925..baaea0ef4 100644
--- a/include/grub/safemath.h
+++ b/include/grub/safemath.h
@@ -32,6 +32,22 @@
  #define grub_cast(a, res)    grub_add ((a), 0, (res))
+#define ALIGN_UP_OVF(v, align, res)             \
+({                            \
+  bool __failed;                    \
+  typeof(v) a;                        \

I think "a" should be changed to some unique name like "__a", because variable shadowing might occur:

int a = 10, b = 4;
if (!ALIGN_UP_OVF(a, b, &a)) { // "a" is shadowed
   printf("a=%d\n", a);
}

The uniqueness of variable name can reduce the possibility of variable shadowing, but it will not eliminate the pitfall.
https://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html

The ultimate solution would be a macro-like "static inline" function, but I'm not sure whether a inline function is appropriate in grub headers.


Oh, sorry, I'm wrong. This marco can't be a inline function because function signature requires specific types to be given to a variable, which will eliminate the efforts of overflow macros.

By the way, I think it would be better to write a comment about "align should not equal to 0 and it must be a power of 2"

Zhang Boyang

+                            \
+  __failed = grub_sub ((typeof(v))(align), 1, &a);    \

The cast "(typeof(v))" is unnecessary and it will silently ignore overflow/underflow.

Zhang Boyang

+  if (__failed == false)                \
+    {                            \
+    __failed = grub_add (v, a, res);            \
+    if (__failed == false)                \
+      {                            \
+        *(res) &= ~a;                    \
+      }                            \
+    }                            \
+__failed;})
+
  #else
  #error gcc 5.1 or newer or clang 8.0 or newer is required
  #endif

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

Reply via email to