Messages in this thread | | | Date | Mon, 15 Dec 2008 12:02:38 +0100 | From | Oleg Nesterov <> | Subject | Re: broken do_each_pid_{thread,task} |
| |
On 12/15, Jiri Slaby wrote: > > Oleg Nesterov napsal(a): > > On 12/14, Jiri Slaby wrote: > >> I'm getting > >> `if (type == PIDTYPE_PID)' is unreachable > >> warning from kernel/exit.c. The preprocessed code looks like: > >> do { > >> struct hlist_node *pos___; > >> if (pgrp != ((void *)0)) > >> for (LIST ITERATION) { > >> { > >> if (!((p->state & 4) != 0)) > >> continue; > >> retval = 1; > >> break; > >> } > >> if (PIDTYPE_PGID == PIDTYPE_PID) > >> break; > >> } > >> } while (0); > >> and it's obviously wrong. > > > > Why do you think it is wrong? This break stops the "hlist_for_each" > > loop, not the enclosing "do while". > > The `continue' matters here (and also in other do_each_pid_task cases). > Sorry for not mentioning it explicitly.
Still can't understand... OK, I think we misundersood each other. Do you agree that the code is technically correct? Or I missed something?
"continue" looks fine to me too, it is also for the inner loop.
> > Actually, I don't understand why the compiler complains, and I never > > saw a warning myself. > > Because the `if' is not reachable :).
Yes, I see it is not reachable, but I don't understand why this deserves a warning ;)
Look, "if (PIDTYPE_PGID == PIDTYPE_PID)" is not possible too, should the compiler (or whatever) complain? > (And it's not compiler which complains > here.)
Ah, OK, thanks. Just curious, and who does?
> > Yes, this is obviously not what was intended. But afaics, this is > > the only place which should be fixed? > > Actually yes. And add a big warning to the macros or whatever to not get > into it later again.
Agreed.
Oleg.
| |