Re: [PATCH 1/4] mm/gup: add compound page list iterator

From: Jason Gunthorpe
Date: Thu Feb 04 2021 - 15:17:55 EST


On Wed, Feb 03, 2021 at 03:00:01PM -0800, John Hubbard wrote:
> > +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.

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

Jason