ewah/bitmap: silence warning about MASK macro redefinition

PATCH In review Eric Sunshine 3 participants 10 messages Jun 15, 2016

Share & tools

Eric Sunshine

author

2016-06-15 23:05 UTC #0 lore raw

PATCH 1/1 ewah/bitmap: silence warning about MASK macro redefinition

On PowerPC Mac OS X (10.5.8 "Leopard" with Xcode 3.1), system header /usr/include/ppc/param.h[1] pollutes the preprocessor namespace with a macro generically named MASK. This conflicts with the same-named macro in ewah/bitmap.c.

[1]: Included indirectly via:

     git-compat-util.h ->
     sys/sysctl.h ->
     sys/ucred.h ->
     sys/param.h ->
     machine/param.h ->
     ppc/param.h

Signed-off-by Eric Sunshine <[email protected]>

---

The alternative is to rename MASK in ewah/bitmap.c to something less generic, resulting in a slightly more noisy patch. I chose the #undef approach since it's a relatively common idiom to #undef a macro before #defining it in order to avoid exactly this sort of redefinition problem.

 ewah/bitmap.c | 2 ++
 1 file changed, 2 insertions(+)

ewah/bitmap.c +2 -0

index 710e58c..b31cf75 100644
@@ -20,6 +20,8 @@
 #include "git-compat-util.h"
 #include "ewok.h"
 
+#undef MASK
+#undef BLOCK
 #define MASK(x) ((eword_t)1 << (x % BITS_IN_WORD))
 #define BLOCK(x) (x / BITS_IN_WORD)

Signature · 1 line

2.4.2.598.gb4379f4

Junio C Hamano

2016-06-15 23:05 UTC #1 lore raw

Quoted 13 lines · Eric Sunshine

On PowerPC Mac OS X (10.5.8 "Leopard" with Xcode 3.1), system header /usr/include/ppc/param.h[1] pollutes the preprocessor namespace with a macro generically named MASK. This conflicts with the same-named macro in ewah/bitmap.c.
[1]: Included indirectly via:
     git-compat-util.h ->
     sys/sysctl.h ->
     sys/ucred.h ->
     sys/param.h ->
     machine/param.h ->
     ppc/param.h

Quoted 8 lines

Signed-off-by Eric Sunshine <[email protected]>
---
The alternative is to rename MASK in ewah/bitmap.c to something less generic, resulting in a slightly more noisy patch. I chose the #undef approach since it's a relatively common idiom to #undef a macro before #defining it in order to avoid exactly this sort of redefinition problem.

I agree that there is nothing controversial against this use of #undef; I however wonder if we are stomping on _their_ use of MASK and breaking whatever Mac OS X wants to express with that macro, though (in which case, hiding collision like this may not be sufficient and we have to live with the poor implementation choice made by the header file and change the macro _we_ use).

Will queue. Thanks.

Quoted 15 lines

 ewah/bitmap.c | 2 ++
 1 file changed, 2 insertions(+)

ewah/bitmap.c +2 -0

index 710e58c..b31cf75 100644
@@ -20,6 +20,8 @@
 #include "git-compat-util.h"
 #include "ewok.h"
 
+#undef MASK
+#undef BLOCK
 #define MASK(x) ((eword_t)1 << (x % BITS_IN_WORD))
 #define BLOCK(x) (x / BITS_IN_WORD)

Jeff King

2016-06-15 23:05 UTC #2 lore raw

Quoted 6 lines · Junio C Hamano

Quoted 5 lines
The alternative is to rename MASK in ewah/bitmap.c to something less generic, resulting in a slightly more noisy patch. I chose the #undef approach since it's a relatively common idiom to #undef a macro before #defining it in order to avoid exactly this sort of redefinition problem.

Quoted 6 lines

I agree that there is nothing controversial against this use of #undef; I however wonder if we are stomping on _their_ use of MASK and breaking whatever Mac OS X wants to express with that macro, though (in which case, hiding collision like this may not be sufficient and we have to live with the poor implementation choice made by the header file and change the macro _we_ use).

Yeah. It looks like we only use the macros a few times. I would not be opposed to s/(BLOCK|MASK)/EWAH_&/. We also use BITS_IN_WORD, which is rather generic, and is actually defined in a header file (so it affects the rest of git). Probably BITS_IN_EWORD would be better.

The resulting patch is rather noisy, but I don't think there's any ongoing work in that area (actually, I have a few things that need cleaned up and submitted, but the conflicts are not a big deal).

-- >8 --
Subject: [PATCH] ewah: use less generic macro names

The ewah/ewok.h header pollutes the global namespace with "BITS_IN_WORD", without any specific notion that we are talking about the bits in an eword_t. We can give this the more specific name "BITS_IN_EWORD".

Likewise, ewah/bitmap.c uses the generic MASK and BLOCK macro names. These are local to the .c file, but we have the opposite problem: on PowerPC Mac OS X (10.5.8 "Leopard" with Xcode 3.1), system header /usr/include/ppc/param.h[1] pollutes the preprocessor namespace with a macro generically named MASK. We can give these macros more specific names, as well, to avoid this conflict.

Reported-and-analyzed-by Eric Sunshine <[email protected]>

Signed-off-by Jeff King <[email protected]>

---
I'm also happy to split it into two patches, and make Eric the author on the MASK part.

 ewah/bitmap.c      | 24 ++++++++++++------------
 ewah/ewah_bitmap.c | 22 +++++++++++-----------
 ewah/ewok.h        |  2 +-
 pack-bitmap.c      | 10 +++++-----
 4 files changed, 29 insertions(+), 29 deletions(-)

ewah/bitmap.c +12 ~~-12~~

index 710e58c..47ad674 100644
@@ -20,8 +20,8 @@
 #include "git-compat-util.h"
 #include "ewok.h"
 
-#define MASK(x) ((eword_t)1 << (x % BITS_IN_WORD))
-#define BLOCK(x) (x / BITS_IN_WORD)
+#define EWAH_MASK(x) ((eword_t)1 << (x % BITS_IN_EWORD))
+#define EWAH_BLOCK(x) (x / BITS_IN_EWORD)
 
 struct bitmap *bitmap_new(void)
 {

@@ -33,7 +33,7 @@ struct bitmap *bitmap_new(void)
 
 void bitmap_set(struct bitmap *self, size_t pos)
 {
-	size_t block = BLOCK(pos);
+	size_t block = EWAH_BLOCK(pos);
 
 	if (block >= self->word_alloc) {
 		size_t old_size = self->word_alloc;

@@ -45,22 +45,22 @@ void bitmap_set(struct bitmap *self, size_t pos)
 			(self->word_alloc - old_size) * sizeof(eword_t));
 	}
 
-	self->words[block] |= MASK(pos);
+	self->words[block] |= EWAH_MASK(pos);
 }
 
 void bitmap_clear(struct bitmap *self, size_t pos)
 {
-	size_t block = BLOCK(pos);
+	size_t block = EWAH_BLOCK(pos);
 
 	if (block < self->word_alloc)
-		self->words[block] &= ~MASK(pos);
+		self->words[block] &= ~EWAH_MASK(pos);
 }
 
 int bitmap_get(struct bitmap *self, size_t pos)
 {
-	size_t block = BLOCK(pos);
+	size_t block = EWAH_BLOCK(pos);
 	return block < self->word_alloc &&
-		(self->words[block] & MASK(pos)) != 0;
+		(self->words[block] & EWAH_MASK(pos)) != 0;
 }
 
 struct ewah_bitmap *bitmap_to_ewah(struct bitmap *bitmap)

@@ -127,7 +127,7 @@ void bitmap_and_not(struct bitmap *self, struct bitmap *other)
 void bitmap_or_ewah(struct bitmap *self, struct ewah_bitmap *other)
 {
 	size_t original_size = self->word_alloc;
-	size_t other_final = (other->bit_size / BITS_IN_WORD) + 1;
+	size_t other_final = (other->bit_size / BITS_IN_EWORD) + 1;
 	size_t i = 0;
 	struct ewah_iterator it;
 	eword_t word;

@@ -155,17 +155,17 @@ void bitmap_each_bit(struct bitmap *self, ewah_callback callback, void *data)
 		uint32_t offset;
 
 		if (word == (eword_t)~0) {
-			for (offset = 0; offset < BITS_IN_WORD; ++offset)
+			for (offset = 0; offset < BITS_IN_EWORD; ++offset)
 				callback(pos++, data);
 		} else {
-			for (offset = 0; offset < BITS_IN_WORD; ++offset) {
+			for (offset = 0; offset < BITS_IN_EWORD; ++offset) {
 				if ((word >> offset) == 0)
 					break;
 
 				offset += ewah_bit_ctz64(word >> offset);
 				callback(pos + offset, data);
 			}
-			pos += BITS_IN_WORD;
+			pos += BITS_IN_EWORD;
 		}
 	}
 }

ewah/ewah_bitmap.c +11 ~~-11~~

index fccb42b..b522437 100644
@@ -102,7 +102,7 @@ size_t ewah_add_empty_words(struct ewah_bitmap *self, int v, size_t number)
 	if (number == 0)
 		return 0;
 
-	self->bit_size += number * BITS_IN_WORD;
+	self->bit_size += number * BITS_IN_EWORD;
 	return add_empty_words(self, v, number);
 }

@@ -152,7 +152,7 @@ void ewah_add_dirty_words(
 			self->buffer_size += can_add;
 		}
 
-		self->bit_size += can_add * BITS_IN_WORD;
+		self->bit_size += can_add * BITS_IN_EWORD;
 
 		if (number - can_add == 0)
 			break;

@@ -197,7 +197,7 @@ static size_t add_empty_word(struct ewah_bitmap *self, int v)
 
 size_t ewah_add(struct ewah_bitmap *self, eword_t word)
 {
-	self->bit_size += BITS_IN_WORD;
+	self->bit_size += BITS_IN_EWORD;
 
 	if (word == 0)
 		return add_empty_word(self, 0);

@@ -211,8 +211,8 @@ size_t ewah_add(struct ewah_bitmap *self, eword_t word)
 void ewah_set(struct ewah_bitmap *self, size_t i)
 {
 	const size_t dist =
-		(i + BITS_IN_WORD) / BITS_IN_WORD -
-		(self->bit_size + BITS_IN_WORD - 1) / BITS_IN_WORD;
+		(i + BITS_IN_EWORD) / BITS_IN_EWORD -
+		(self->bit_size + BITS_IN_EWORD - 1) / BITS_IN_EWORD;
 
 	assert(i >= self->bit_size);

@@ -222,19 +222,19 @@ void ewah_set(struct ewah_bitmap *self, size_t i)
 		if (dist > 1)
 			add_empty_words(self, 0, dist - 1);
 
-		add_literal(self, (eword_t)1 << (i % BITS_IN_WORD));
+		add_literal(self, (eword_t)1 << (i % BITS_IN_EWORD));
 		return;
 	}
 
 	if (rlw_get_literal_words(self->rlw) == 0) {
 		rlw_set_running_len(self->rlw,
 			rlw_get_running_len(self->rlw) - 1);
-		add_literal(self, (eword_t)1 << (i % BITS_IN_WORD));
+		add_literal(self, (eword_t)1 << (i % BITS_IN_EWORD));
 		return;
 	}
 
 	self->buffer[self->buffer_size - 1] |=
-		((eword_t)1 << (i % BITS_IN_WORD));
+		((eword_t)1 << (i % BITS_IN_EWORD));
 
 	/* check if we just completed a stream of 1s */
 	if (self->buffer[self->buffer_size - 1] == (eword_t)(~0)) {

@@ -255,11 +255,11 @@ void ewah_each_bit(struct ewah_bitmap *self, void (*callback)(size_t, void*), vo
 		eword_t *word = &self->buffer[pointer];
 
 		if (rlw_get_run_bit(word)) {
-			size_t len = rlw_get_running_len(word) * BITS_IN_WORD;
+			size_t len = rlw_get_running_len(word) * BITS_IN_EWORD;
 			for (k = 0; k < len; ++k, ++pos)
 				callback(pos, payload);
 		} else {
-			pos += rlw_get_running_len(word) * BITS_IN_WORD;
+			pos += rlw_get_running_len(word) * BITS_IN_EWORD;
 		}
 
 		++pointer;

@@ -268,7 +268,7 @@ void ewah_each_bit(struct ewah_bitmap *self, void (*callback)(size_t, void*), vo
 			int c;
 
 			/* todo: zero count optimization */
-			for (c = 0; c < BITS_IN_WORD; ++c, ++pos) {
+			for (c = 0; c < BITS_IN_EWORD; ++c, ++pos) {
 				if ((self->buffer[pointer] & ((eword_t)1 << c)) != 0)
 					callback(pos, payload);
 			}

ewah/ewok.h +1 -1

index e732525..6e2c5e1 100644
@@ -32,7 +32,7 @@
 
 struct strbuf;
 typedef uint64_t eword_t;
-#define BITS_IN_WORD (sizeof(eword_t) * 8)
+#define BITS_IN_EWORD (sizeof(eword_t) * 8)
 
 /**
  * Do not use __builtin_popcountll. The GCC implementation

pack-bitmap.c +5 -5

index 2b3ff23..637770a 100644
@@ -622,7 +622,7 @@ static void show_objects_for_type(
 	while (i < objects->word_alloc && ewah_iterator_next(&filter, &it)) {
 		eword_t word = objects->words[i] & filter;
 
-		for (offset = 0; offset < BITS_IN_WORD; ++offset) {
+		for (offset = 0; offset < BITS_IN_EWORD; ++offset) {
 			const unsigned char *sha1;
 			struct revindex_entry *entry;
 			uint32_t hash = 0;

@@ -644,7 +644,7 @@ static void show_objects_for_type(
 			show_reach(sha1, object_type, 0, hash, bitmap_git.pack, entry->offset);
 		}
 
-		pos += BITS_IN_WORD;
+		pos += BITS_IN_EWORD;
 		i++;
 	}
 }

@@ -776,7 +776,7 @@ int reuse_partial_packfile_from_bitmap(struct packed_git **packfile,
 			break;
 		}
 
-		reuse_objects += BITS_IN_WORD;
+		reuse_objects += BITS_IN_EWORD;
 	}
 
 #ifdef GIT_BITMAP_DEBUG

@@ -1001,7 +1001,7 @@ static int rebuild_bitmap(uint32_t *reposition,
 	while (ewah_iterator_next(&word, &it)) {
 		uint32_t offset, bit_pos;
 
-		for (offset = 0; offset < BITS_IN_WORD; ++offset) {
+		for (offset = 0; offset < BITS_IN_EWORD; ++offset) {
 			if ((word >> offset) == 0)
 				break;

@@ -1014,7 +1014,7 @@ static int rebuild_bitmap(uint32_t *reposition,
 				return -1;
 		}
 
-		pos += BITS_IN_WORD;
+		pos += BITS_IN_EWORD;
 	}
 	return 0;
 }

Signature · 1 line

2.4.2.745.g0aa058d

Eric Sunshine

author

2016-06-15 23:05 UTC #3 lore raw

Quoted 11 lines · Jeff King

Subject: [PATCH] ewah: use less generic macro names
The ewah/ewok.h header pollutes the global namespace with "BITS_IN_WORD", without any specific notion that we are talking about the bits in an eword_t. We can give this the more specific name "BITS_IN_EWORD".
Likewise, ewah/bitmap.c uses the generic MASK and BLOCK macro names. These are local to the .c file, but we have the opposite problem: on PowerPC Mac OS X (10.5.8 "Leopard" with Xcode 3.1), system header /usr/include/ppc/param.h[1]

The [1] has no meaning anymore, so: s/\[1\]//

Quoted 9 lines

pollutes the preprocessor namespace with a macro generically named MASK. We can give these macros more specific names, as well, to avoid this conflict.
Reported-and-analyzed-by Eric Sunshine <[email protected]>
Signed-off-by Jeff King <[email protected]>
---
I'm also happy to split it into two patches, and make Eric the author on the MASK part.

I don't mind being an author of a patch, so splitting works for me, however, I'm also fine with whatever you and Junio prefer. (And, in retrospect, I agree that renaming the macros here is preferable over the #undef approach.)

Jeff King

2016-06-15 23:05 UTC #4 lore raw

Quoted 8 lines · Eric Sunshine

Quoted 7 lines · Jeff King
Subject: [PATCH] ewah: use less generic macro names
The ewah/ewok.h header pollutes the global namespace with "BITS_IN_WORD", without any specific notion that we are talking about the bits in an eword_t. We can give this the more specific name "BITS_IN_EWORD".

Quoted 6 lines

Quoted 4 lines
Likewise, ewah/bitmap.c uses the generic MASK and BLOCK macro names. These are local to the .c file, but we have the opposite problem: on PowerPC Mac OS X (10.5.8 "Leopard" with Xcode 3.1), system header /usr/include/ppc/param.h[1]
The [1] has no meaning anymore, so: s/\[1\]//

Oops, I meant to include your footnote.

Quoted 7 lines

Quoted 2 lines
I'm also happy to split it into two patches, and make Eric the author on the MASK part.
I don't mind being an author of a patch, so splitting works for me, however, I'm also fine with whatever you and Junio prefer. (And, in retrospect, I agree that renaming the macros here is preferable over the #undef approach.)

After reflecting, I think splitting it into two patches makes more sense. I mostly noticed that we were touching the same lines, but of course it is not a problem to have two related patches in order. :)

So here is what I propose:

[1/2]: ewah/bitmap: silence warning about MASK macro redefinition [2/2]: ewah: use less generic macro name

I put yours first, because it solves an actual problem. We can drop 2/2 if we decide it's just churn.

-Peff

Jeff King

2016-06-15 23:05 UTC #5 lore raw

PATCH 1/2 ewah/bitmap: silence warning about MASK macro redefinition

From: Eric Sunshine <[email protected]>

[1]: Included indirectly via:

     git-compat-util.h ->
     sys/sysctl.h ->
     sys/ucred.h ->
     sys/param.h ->
     machine/param.h ->
     ppc/param.h

Signed-off-by Jeff King <[email protected]>

---
 ewah/bitmap.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

ewah/bitmap.c +8 -8

index 710e58c..2af94f6 100644
@@ -20,8 +20,8 @@
 #include "git-compat-util.h"
 #include "ewok.h"
 
-#define MASK(x) ((eword_t)1 << (x % BITS_IN_WORD))
-#define BLOCK(x) (x / BITS_IN_WORD)
+#define EWAH_MASK(x) ((eword_t)1 << (x % BITS_IN_WORD))
+#define EWAH_BLOCK(x) (x / BITS_IN_WORD)
 
 struct bitmap *bitmap_new(void)
 {

@@ -33,7 +33,7 @@ struct bitmap *bitmap_new(void)
 
 void bitmap_set(struct bitmap *self, size_t pos)
 {
-	size_t block = BLOCK(pos);
+	size_t block = EWAH_BLOCK(pos);
 
 	if (block >= self->word_alloc) {
 		size_t old_size = self->word_alloc;

@@ -45,22 +45,22 @@ void bitmap_set(struct bitmap *self, size_t pos)
 			(self->word_alloc - old_size) * sizeof(eword_t));
 	}
 
-	self->words[block] |= MASK(pos);
+	self->words[block] |= EWAH_MASK(pos);
 }
 
 void bitmap_clear(struct bitmap *self, size_t pos)
 {
-	size_t block = BLOCK(pos);
+	size_t block = EWAH_BLOCK(pos);
 
 	if (block < self->word_alloc)
-		self->words[block] &= ~MASK(pos);
+		self->words[block] &= ~EWAH_MASK(pos);
 }
 
 int bitmap_get(struct bitmap *self, size_t pos)
 {
-	size_t block = BLOCK(pos);
+	size_t block = EWAH_BLOCK(pos);
 	return block < self->word_alloc &&
-		(self->words[block] & MASK(pos)) != 0;
+		(self->words[block] & EWAH_MASK(pos)) != 0;
 }
 
 struct ewah_bitmap *bitmap_to_ewah(struct bitmap *bitmap)

Signature · 1 line

2.4.2.745.g0aa058d

Eric Sunshine

author

2016-06-15 23:05 UTC #6 lore raw

Quoted 9 lines · Jeff King

From: Eric Sunshine <[email protected]>
On PowerPC Mac OS X (10.5.8 "Leopard" with Xcode 3.1), system header /usr/include/ppc/param.h[1] pollutes the preprocessor namespace with a macro generically named MASK. This conflicts with the same-named macro in ewah/bitmap.c. We can avoid this conflict by using a more specific name.
[1]: Included indirectly via:

Quoted 8 lines

     git-compat-util.h ->
     sys/sysctl.h ->
     sys/ucred.h ->
     sys/param.h ->
     machine/param.h ->
     ppc/param.h

Signed-off-by Jeff King <[email protected]>

Signed-off-by Eric Sunshine <[email protected]>

Quoted 58 lines

---
 ewah/bitmap.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

ewah/bitmap.c +8 -9

index 710e58c..2af94f6 100644
@@ -20,8 +20,8 @@
 #include "git-compat-util.h"
 #include "ewok.h"
 
-#define MASK(x) ((eword_t)1 << (x % BITS_IN_WORD))
-#define BLOCK(x) (x / BITS_IN_WORD)
+#define EWAH_MASK(x) ((eword_t)1 << (x % BITS_IN_WORD))
+#define EWAH_BLOCK(x) (x / BITS_IN_WORD)
 
 struct bitmap *bitmap_new(void)
 {

@@ -33,7 +33,7 @@ struct bitmap *bitmap_new(void)
 
 void bitmap_set(struct bitmap *self, size_t pos)
 {
-       size_t block = BLOCK(pos);
+       size_t block = EWAH_BLOCK(pos);
 
        if (block >= self->word_alloc) {
                size_t old_size = self->word_alloc;

@@ -45,22 +45,22 @@ void bitmap_set(struct bitmap *self, size_t pos)
                        (self->word_alloc - old_size) * sizeof(eword_t));
        }
 
-       self->words[block] |= MASK(pos);
+       self->words[block] |= EWAH_MASK(pos);
 }
 
 void bitmap_clear(struct bitmap *self, size_t pos)
 {
-       size_t block = BLOCK(pos);
+       size_t block = EWAH_BLOCK(pos);
 
        if (block < self->word_alloc)
-               self->words[block] &= ~MASK(pos);
+               self->words[block] &= ~EWAH_MASK(pos);
 }
 
 int bitmap_get(struct bitmap *self, size_t pos)
 {
-       size_t block = BLOCK(pos);
+       size_t block = EWAH_BLOCK(pos);
        return block < self->word_alloc &&
-               (self->words[block] & MASK(pos)) != 0;
+               (self->words[block] & EWAH_MASK(pos)) != 0;
 }
 
 struct ewah_bitmap *bitmap_to_ewah(struct bitmap *bitmap)
--

2.4.2.745.g0aa058d

Jeff King

2016-06-15 23:05 UTC #7 lore raw

PATCH 2/2 ewah: use less generic macro name

In reply to Jeff King

Signed-off-by Jeff King <[email protected]>

---
 ewah/bitmap.c      | 12 ++++++------
 ewah/ewah_bitmap.c | 22 +++++++++++-----------
 ewah/ewok.h        |  2 +-
 pack-bitmap.c      | 10 +++++-----
 4 files changed, 23 insertions(+), 23 deletions(-)

ewah/bitmap.c +6 -6

index 2af94f6..47ad674 100644
@@ -20,8 +20,8 @@
 #include "git-compat-util.h"
 #include "ewok.h"
 
-#define EWAH_MASK(x) ((eword_t)1 << (x % BITS_IN_WORD))
-#define EWAH_BLOCK(x) (x / BITS_IN_WORD)
+#define EWAH_MASK(x) ((eword_t)1 << (x % BITS_IN_EWORD))
+#define EWAH_BLOCK(x) (x / BITS_IN_EWORD)
 
 struct bitmap *bitmap_new(void)
 {

@@ -127,7 +127,7 @@ void bitmap_and_not(struct bitmap *self, struct bitmap *other)
 void bitmap_or_ewah(struct bitmap *self, struct ewah_bitmap *other)
 {
 	size_t original_size = self->word_alloc;
-	size_t other_final = (other->bit_size / BITS_IN_WORD) + 1;
+	size_t other_final = (other->bit_size / BITS_IN_EWORD) + 1;
 	size_t i = 0;
 	struct ewah_iterator it;
 	eword_t word;

@@ -155,17 +155,17 @@ void bitmap_each_bit(struct bitmap *self, ewah_callback callback, void *data)
 		uint32_t offset;
 
 		if (word == (eword_t)~0) {
-			for (offset = 0; offset < BITS_IN_WORD; ++offset)
+			for (offset = 0; offset < BITS_IN_EWORD; ++offset)
 				callback(pos++, data);
 		} else {
-			for (offset = 0; offset < BITS_IN_WORD; ++offset) {
+			for (offset = 0; offset < BITS_IN_EWORD; ++offset) {
 				if ((word >> offset) == 0)
 					break;
 
 				offset += ewah_bit_ctz64(word >> offset);
 				callback(pos + offset, data);
 			}
-			pos += BITS_IN_WORD;
+			pos += BITS_IN_EWORD;
 		}
 	}
 }

ewah/ewah_bitmap.c +11 ~~-11~~

index fccb42b..b522437 100644
@@ -102,7 +102,7 @@ size_t ewah_add_empty_words(struct ewah_bitmap *self, int v, size_t number)
 	if (number == 0)
 		return 0;
 
-	self->bit_size += number * BITS_IN_WORD;
+	self->bit_size += number * BITS_IN_EWORD;
 	return add_empty_words(self, v, number);
 }

@@ -152,7 +152,7 @@ void ewah_add_dirty_words(
 			self->buffer_size += can_add;
 		}
 
-		self->bit_size += can_add * BITS_IN_WORD;
+		self->bit_size += can_add * BITS_IN_EWORD;
 
 		if (number - can_add == 0)
 			break;

@@ -197,7 +197,7 @@ static size_t add_empty_word(struct ewah_bitmap *self, int v)
 
 size_t ewah_add(struct ewah_bitmap *self, eword_t word)
 {
-	self->bit_size += BITS_IN_WORD;
+	self->bit_size += BITS_IN_EWORD;
 
 	if (word == 0)
 		return add_empty_word(self, 0);

@@ -211,8 +211,8 @@ size_t ewah_add(struct ewah_bitmap *self, eword_t word)
 void ewah_set(struct ewah_bitmap *self, size_t i)
 {
 	const size_t dist =
-		(i + BITS_IN_WORD) / BITS_IN_WORD -
-		(self->bit_size + BITS_IN_WORD - 1) / BITS_IN_WORD;
+		(i + BITS_IN_EWORD) / BITS_IN_EWORD -
+		(self->bit_size + BITS_IN_EWORD - 1) / BITS_IN_EWORD;
 
 	assert(i >= self->bit_size);

@@ -222,19 +222,19 @@ void ewah_set(struct ewah_bitmap *self, size_t i)
 		if (dist > 1)
 			add_empty_words(self, 0, dist - 1);
 
-		add_literal(self, (eword_t)1 << (i % BITS_IN_WORD));
+		add_literal(self, (eword_t)1 << (i % BITS_IN_EWORD));
 		return;
 	}
 
 	if (rlw_get_literal_words(self->rlw) == 0) {
 		rlw_set_running_len(self->rlw,
 			rlw_get_running_len(self->rlw) - 1);
-		add_literal(self, (eword_t)1 << (i % BITS_IN_WORD));
+		add_literal(self, (eword_t)1 << (i % BITS_IN_EWORD));
 		return;
 	}
 
 	self->buffer[self->buffer_size - 1] |=
-		((eword_t)1 << (i % BITS_IN_WORD));
+		((eword_t)1 << (i % BITS_IN_EWORD));
 
 	/* check if we just completed a stream of 1s */
 	if (self->buffer[self->buffer_size - 1] == (eword_t)(~0)) {

@@ -255,11 +255,11 @@ void ewah_each_bit(struct ewah_bitmap *self, void (*callback)(size_t, void*), vo
 		eword_t *word = &self->buffer[pointer];
 
 		if (rlw_get_run_bit(word)) {
-			size_t len = rlw_get_running_len(word) * BITS_IN_WORD;
+			size_t len = rlw_get_running_len(word) * BITS_IN_EWORD;
 			for (k = 0; k < len; ++k, ++pos)
 				callback(pos, payload);
 		} else {
-			pos += rlw_get_running_len(word) * BITS_IN_WORD;
+			pos += rlw_get_running_len(word) * BITS_IN_EWORD;
 		}
 
 		++pointer;

@@ -268,7 +268,7 @@ void ewah_each_bit(struct ewah_bitmap *self, void (*callback)(size_t, void*), vo
 			int c;
 
 			/* todo: zero count optimization */
-			for (c = 0; c < BITS_IN_WORD; ++c, ++pos) {
+			for (c = 0; c < BITS_IN_EWORD; ++c, ++pos) {
 				if ((self->buffer[pointer] & ((eword_t)1 << c)) != 0)
 					callback(pos, payload);
 			}

ewah/ewok.h +1 -1

index e732525..6e2c5e1 100644
@@ -32,7 +32,7 @@
 
 struct strbuf;
 typedef uint64_t eword_t;
-#define BITS_IN_WORD (sizeof(eword_t) * 8)
+#define BITS_IN_EWORD (sizeof(eword_t) * 8)
 
 /**
  * Do not use __builtin_popcountll. The GCC implementation

pack-bitmap.c +5 -5

index 2b3ff23..637770a 100644
@@ -622,7 +622,7 @@ static void show_objects_for_type(
 	while (i < objects->word_alloc && ewah_iterator_next(&filter, &it)) {
 		eword_t word = objects->words[i] & filter;
 
-		for (offset = 0; offset < BITS_IN_WORD; ++offset) {
+		for (offset = 0; offset < BITS_IN_EWORD; ++offset) {
 			const unsigned char *sha1;
 			struct revindex_entry *entry;
 			uint32_t hash = 0;

@@ -644,7 +644,7 @@ static void show_objects_for_type(
 			show_reach(sha1, object_type, 0, hash, bitmap_git.pack, entry->offset);
 		}
 
-		pos += BITS_IN_WORD;
+		pos += BITS_IN_EWORD;
 		i++;
 	}
 }

@@ -776,7 +776,7 @@ int reuse_partial_packfile_from_bitmap(struct packed_git **packfile,
 			break;
 		}
 
-		reuse_objects += BITS_IN_WORD;
+		reuse_objects += BITS_IN_EWORD;
 	}
 
 #ifdef GIT_BITMAP_DEBUG

@@ -1001,7 +1001,7 @@ static int rebuild_bitmap(uint32_t *reposition,
 	while (ewah_iterator_next(&word, &it)) {
 		uint32_t offset, bit_pos;
 
-		for (offset = 0; offset < BITS_IN_WORD; ++offset) {
+		for (offset = 0; offset < BITS_IN_EWORD; ++offset) {
 			if ((word >> offset) == 0)
 				break;

@@ -1014,7 +1014,7 @@ static int rebuild_bitmap(uint32_t *reposition,
 				return -1;
 		}
 
-		pos += BITS_IN_WORD;
+		pos += BITS_IN_EWORD;
 	}
 	return 0;
 }

Signature · 1 line

2.4.2.745.g0aa058d

Eric Sunshine

author Reviewed-by on v1 2/2

2016-06-15 23:05 UTC #8 lore raw

Re: [PATCH 2/2] ewah: use less generic macro name

Quoted 6 lines · Jeff King

The ewah/ewok.h header pollutes the global namespace with "BITS_IN_WORD", without any specific notion that we are talking about the bits in an eword_t. We can give this the more specific name "BITS_IN_EWORD".
Signed-off-by Jeff King <[email protected]>

Yeah, splitting the patches this way makes sense and feels a bit cleaner.

It's such a simple patch that it doesn't need a reviewed-by, but for what it's worth...

Reviewed-by Eric Sunshine <[email protected]>

Junio C Hamano

2016-06-15 23:05 UTC #9 lore raw

In reply to Eric Sunshine

Quoted 6 lines · Eric Sunshine

Quoted 3 lines · Jeff King
---
I'm also happy to split it into two patches, and make Eric the author on the MASK part.
I don't mind being an author of a patch, so splitting works for me,

OK, then let's have two patches, one from you each.

Thanks.