<div dir="ltr"><div dir="ltr"><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, Sep 17, 2018 at 11:27 PM Richard W.M. Jones <<a href="mailto:rjones@redhat.com">rjones@redhat.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">---<br>
 common/include/isaligned.h | 11 +++++------<br>
 plugins/file/file.c        |  4 ++--<br>
 plugins/vddk/vddk.c        |  8 ++++----<br>
 3 files changed, 11 insertions(+), 12 deletions(-)<br>
<br>
diff --git a/common/include/isaligned.h b/common/include/isaligned.h<br>
index e693820..81ce8a7 100644<br>
--- a/common/include/isaligned.h<br>
+++ b/common/include/isaligned.h<br>
@@ -36,16 +36,15 @@<br>
<br>
 #include <assert.h><br>
 #include <stdbool.h><br>
+#include <stdint.h><br></blockquote><div><br></div><div>Not need with this change...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
 #include "ispowerof2.h"<br>
<br>
 /* Return true if size is a multiple of align. align must be power of 2.<br>
  */<br>
-static inline bool<br>
-is_aligned (unsigned int size, unsigned int align)<br>
-{<br>
-  assert (is_power_of_2 (align));<br>
-  return !(size & (align - 1));<br>
-}<br>
+#define IS_ALIGNED(size, align) ({              \<br>
+      assert (is_power_of_2 ((align)));         \<br>
+      !((size) & ((align) - 1));                \<br>
+})<br></blockquote><div><br></div><div>But this version will happily accept singed int, and I think this code </div><div>behavior with signed int is undefined.</div><div><br></div><div>See <a href="https://wiki.sei.cmu.edu/confluence/display/c/INT13-C.+Use+bitwise+operators+only+on+unsigned+operands">https://wiki.sei.cmu.edu/confluence/display/c/INT13-C.+Use+bitwise+operators+only+on+unsigned+operands</a></div><div><br></div><div>Why use a macro when we can use a function that is likely to be inlined anyway?</div><div>This is not used in tight loops, so there is no reason to use a macro.</div><div><br></div><div>If you want to use a macro, see the kernel align macros:</div><div><br></div><div><div>  57 /* @a is a power of 2 value */</div><div>  58 #define ALIGN(x, a)     __ALIGN_KERNEL((x), (a))</div><div>  59 #define ALIGN_DOWN(x, a)    __ALIGN_KERNEL((x) - ((a) - 1), (a))</div><div>  60 #define __ALIGN_MASK(x, mask)   __ALIGN_KERNEL_MASK((x), (mask))</div><div>  61 #define PTR_ALIGN(p, a)     ((typeof(p))ALIGN((unsigned long)(p), (a)))</div><div>  62 #define IS_ALIGNED(x, a)        (((x) & ((typeof(x))(a) - 1)) == 0)</div></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
 #endif /* NBDKIT_ISALIGNED_H */<br>
diff --git a/plugins/file/file.c b/plugins/file/file.c<br>
index 9d03f18..1391f62 100644<br>
--- a/plugins/file/file.c<br>
+++ b/plugins/file/file.c<br>
@@ -397,13 +397,13 @@ file_zero (void *handle, uint32_t count, uint64_t offset, int may_trim)<br>
<br>
 #ifdef BLKZEROOUT<br>
   /* For aligned range and block device, we can use BLKZEROOUT. */<br>
-  if (h->can_zeroout && is_aligned (offset | count, h->sector_size)) {<br>
+  if (h->can_zeroout && IS_ALIGNED (offset | count, h->sector_size)) {<br>
     uint64_t range[2] = {offset, count};<br>
<br>
     r = ioctl (h->fd, BLKZEROOUT, &range);<br>
     if (r == 0) {<br>
       if (file_debug_zero)<br>
-        nbdkit_debug ("h->can_zeroout && is_aligned: "<br>
+        nbdkit_debug ("h->can_zeroout && IS_ALIGNED: "<br>
                       "zero succeeded using BLKZEROOUT");<br>
       return 0;<br>
     }<br>
diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c<br>
index f3b4539..9bfd4d2 100644<br>
--- a/plugins/vddk/vddk.c<br>
+++ b/plugins/vddk/vddk.c<br>
@@ -511,11 +511,11 @@ vddk_pread (void *handle, void *buf, uint32_t count, uint64_t offset)<br>
   VixError err;<br>
<br>
   /* Align to sectors. */<br>
-  if (!is_aligned (offset, VIXDISKLIB_SECTOR_SIZE)) {<br>
+  if (!IS_ALIGNED (offset, VIXDISKLIB_SECTOR_SIZE)) {<br>
     nbdkit_error ("read is not aligned to sectors");<br>
     return -1;<br>
   }<br>
-  if (!is_aligned (count, VIXDISKLIB_SECTOR_SIZE)) {<br>
+  if (!IS_ALIGNED (count, VIXDISKLIB_SECTOR_SIZE)) {<br>
     nbdkit_error ("read is not aligned to sectors");<br>
     return -1;<br>
   }<br>
@@ -544,11 +544,11 @@ vddk_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset)<br>
   VixError err;<br>
<br>
   /* Align to sectors. */<br>
-  if (!is_aligned (offset, VIXDISKLIB_SECTOR_SIZE)) {<br>
+  if (!IS_ALIGNED (offset, VIXDISKLIB_SECTOR_SIZE)) {<br>
     nbdkit_error ("read is not aligned to sectors");<br>
     return -1;<br>
   }<br>
-  if (!is_aligned (count, VIXDISKLIB_SECTOR_SIZE)) {<br>
+  if (!IS_ALIGNED (count, VIXDISKLIB_SECTOR_SIZE)) {<br>
     nbdkit_error ("read is not aligned to sectors");<br>
     return -1;<br>
   }<br>
-- <br>
2.19.0.rc0<br>
<br>
</blockquote></div></div></div></div>