On 2/4/21 11:53 AM, Jason Gunthorpe wrote:

On Wed, Feb 03, 2021 at 03:00:01PM -0800, John Hubbard wrote:

quoted

quoted

+static inline void compound_next(unsigned long i, unsigned long npages,
+				 struct page **list, struct page **head,
+				 unsigned int *ntails)
+{
+	if (i >= npages)
+		return;
+
+	*ntails = count_ntails(list + i, npages - i);
+	*head = compound_head(list[i]);
+}
+
+#define for_each_compound_head(i, list, npages, head, ntails) \

When using macros, which are dangerous in general, you have to worry about
things like name collisions. I really dislike that C has forced this unsafe
pattern upon us, but of course we are stuck with it, for iterator helpers.

Given that we're stuck, you should probably use names such as __i, __list, etc,
in the the above #define. Otherwise you could stomp on existing variables.

Not this macro, it after cpp gets through with it all the macro names
vanish, it can't collide with variables.

Yes, I guess it does just vaporize, because it turns all the args into
their substituted values. I was just having flashbacks from similar cases
I guess.

The usual worry is you might collide with other #defines, but we don't
seem to worry about that in the kernel

Well, I worry about it a little anyway. haha :)


thanks,
-- 
John Hubbard
NVIDIA