Messages in this thread | | | Date | Mon, 15 Dec 2008 12:33:01 +0100 | From | Jiri Slaby <> | Subject | Re: broken do_each_pid_{thread,task} |
| |
Oleg Nesterov napsal(a): > 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.
But it doesn't jump to the `if' (this is what I would expect from the `continue' here), but to the third statement of the `for'.
Maybe better to ask, is the test expected to be fired after *each* invocation of the body?
>>> 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?
Correct, in this particular case (and I checked that also other users which uses `continue' inside the loop don't pass PIDTYPE_PID).
>> (And it's not compiler which complains >> here.) > > Ah, OK, thanks. Just curious, and who does?
A static analyzer. Stay tuned, we will announce it later, it's in the state of development :).
| |