On Mon, Sep 17, 2018 at 11:27 PM Richard W.M. Jones <rjones@redhat.com> wrote:

---
common/include/isaligned.h | 11 +++++------
plugins/file/file.c | 4 ++--
plugins/vddk/vddk.c | 8 ++++----
3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/common/include/isaligned.h b/common/include/isaligned.h
index e693820..81ce8a7 100644
--- a/common/include/isaligned.h
+++ b/common/include/isaligned.h
@@ -36,16 +36,15 @@

#include <assert.h>
#include <stdbool.h>
+#include <stdint.h>

Not need with this change...

#include "ispowerof2.h"

/* Return true if size is a multiple of align. align must be power of 2.
*/
-static inline bool
-is_aligned (unsigned int size, unsigned int align)
-{
- assert (is_power_of_2 (align));
- return !(size & (align - 1));
-}
+#define IS_ALIGNED(size, align) ({ \
+ assert (is_power_of_2 ((align))); \
+ !((size) & ((align) - 1)); \
+})

But this version will happily accept singed int, and I think this code

behavior with signed int is undefined.

See https://wiki.sei.cmu.edu/confluence/display/c/INT13-C.+Use+bitwise+operators+only+on+unsigned+operands

Why use a macro when we can use a function that is likely to be inlined anyway?

This is not used in tight loops, so there is no reason to use a macro.

If you want to use a macro, see the kernel align macros:

57 /* @a is a power of 2 value */

58 #define ALIGN(x, a) __ALIGN_KERNEL((x), (a))

59 #define ALIGN_DOWN(x, a) __ALIGN_KERNEL((x) - ((a) - 1), (a))

60 #define __ALIGN_MASK(x, mask) __ALIGN_KERNEL_MASK((x), (mask))

61 #define PTR_ALIGN(p, a) ((typeof(p))ALIGN((unsigned long)(p), (a)))

62 #define IS_ALIGNED(x, a) (((x) & ((typeof(x))(a) - 1)) == 0)

#endif /* NBDKIT_ISALIGNED_H */
diff --git a/plugins/file/file.c b/plugins/file/file.c
index 9d03f18..1391f62 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -397,13 +397,13 @@ file_zero (void *handle, uint32_t count, uint64_t offset, int may_trim)

#ifdef BLKZEROOUT
/* For aligned range and block device, we can use BLKZEROOUT. */
- if (h->can_zeroout && is_aligned (offset | count, h->sector_size)) {
+ if (h->can_zeroout && IS_ALIGNED (offset | count, h->sector_size)) {
uint64_t range[2] = {offset, count};

r = ioctl (h->fd, BLKZEROOUT, &range);
if (r == 0) {
if (file_debug_zero)
- nbdkit_debug ("h->can_zeroout && is_aligned: "
+ nbdkit_debug ("h->can_zeroout && IS_ALIGNED: "
"zero succeeded using BLKZEROOUT");
return 0;
}
diff --git a/plugins/vddk/vddk.c b/plugins/vddk/vddk.c
index f3b4539..9bfd4d2 100644
--- a/plugins/vddk/vddk.c
+++ b/plugins/vddk/vddk.c
@@ -511,11 +511,11 @@ vddk_pread (void *handle, void *buf, uint32_t count, uint64_t offset)
VixError err;

/* Align to sectors. */
- if (!is_aligned (offset, VIXDISKLIB_SECTOR_SIZE)) {
+ if (!IS_ALIGNED (offset, VIXDISKLIB_SECTOR_SIZE)) {
nbdkit_error ("read is not aligned to sectors");
return -1;
}
- if (!is_aligned (count, VIXDISKLIB_SECTOR_SIZE)) {
+ if (!IS_ALIGNED (count, VIXDISKLIB_SECTOR_SIZE)) {
nbdkit_error ("read is not aligned to sectors");
return -1;
}
@@ -544,11 +544,11 @@ vddk_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset)
VixError err;

/* Align to sectors. */
- if (!is_aligned (offset, VIXDISKLIB_SECTOR_SIZE)) {
+ if (!IS_ALIGNED (offset, VIXDISKLIB_SECTOR_SIZE)) {
nbdkit_error ("read is not aligned to sectors");
return -1;
}
- if (!is_aligned (count, VIXDISKLIB_SECTOR_SIZE)) {
+ if (!IS_ALIGNED (count, VIXDISKLIB_SECTOR_SIZE)) {
nbdkit_error ("read is not aligned to sectors");
return -1;
}
--
2.19.0.rc0