https://gcc.gnu.org/bugzilla/show_bug.cgi?id=103964
--- Comment #10 from Ilya Maximets <i.maximets at ovn dot org> ---
(In reply to Jakub Jelinek from comment #9)
> (In reply to Ilya Maximets from comment #7)
> > (In reply to Jakub Jelinek from comment #6)
> > > What is the reason why OVS (and kernel) doesn't use 2 variables, one for
> > > the
> > > iterator that is a pointer to the prev/next structure only and one
> > > assigned
> > > e.g. in the condition from the iterator that is used only when it isn't
> > > the
> > > start?
> > > At least if targetting C99 and above (or C++) one can declare one of those
> > > iterators in the for loop init expression...
> >
> > The problem is that we need 2 variables and one of them need to be
> > accessible
> > outside of the loop. And I don't think it is possible to declare one
> > variable
> > and only initialize another one.
>
> If you only need to use one variable outside of the loop and not the other
> one,
> that should be doable. If you use one in some spots and another in other
> spots but never both, you could have different macros for those 2 versions.
>
> E.g. the one where the list iterator is used inside of the loop only and the
> var pointing to the containing objects could look like:
> for (struct ovs_list *iter = (pos = NULL, &start); iter != (&start)
> && (((pos) = ((typeof(pos))(void*)((uintptr_t)((iter->next) -
> __builtin_offsetof(struct member,
> elem))))), 1);
> iter = iter->next, pos = NULL)
> which would get you after the loop pos = NULL if break; wasn't done and
> pos non-NULL otherwise.
Hmm. I missed the possibility of a comma trick in the initialization part.
I think, we can try to re-write our macros this way. Thanks!
> But in your testcase you actually need to use the other var after the loop,
> so it would need to be done the other way around, but then the user variable
> would need to be struct ovs_list * and the macro would need to be told in a
> different way what type the var declared in the loop should have.
Doesn't sound very intuitive. I guess, it's easier to just add a NULL pointer
check after the loop, i.e. ovs_list_insert(pos ? &pos->elem : &start, ...).
Assuming our unit tests will catch all NULL-pointer dereferences (they will
likely not, but still), that could be a semi-automated way to find all the
problematic code parts.
Not very friendly for dependent projects though.
>
> > One thing that is not clear to me is if the following code has an UB or not:
> >
> > struct member* pos;
> > struct ovs_list start;
> >
> > pos = (struct member *)(void*)((uintptr_t)(&start) - 64);
> > ovs_list_insert((void *)((uintptr_t)pos + 64), &member->elem);
>
> It still creates a pointer out of something that doesn't point to such an
> object or points to some unrelated one, doesn't necessarily have sufficient
> alignment etc., so I think it is UB too, but perhaps the compiler at least
> now will handle it the way you expect. A lot of programs including e.g.
> POSIX rely on at least
> (void *) -1 and similar pointer constants to be usable in equality
> comparisons (e.g. MAP_FAILED macro).
We also have this kind of stuff:
static const struct ovs_list OVS_LIST_POISON =
{ (struct ovs_list *) (UINTPTR_MAX / 0xf * 0xc),
(struct ovs_list *) (UINTPTR_MAX / 0xf * 0xc) };
