Well depending on which IRLQ the while loop is in means it could occasionally loose CPU time only later to be rescheduled.
Also, if you do happen to want examine the running code path, you need to break in to the running OS, find the runaway thread and attach to it. Having an ASSERT(FALSE) is surely much simpler?
I’m sure Alex had his reasons for using while (TRUE), but I’m not sure what they are.
Ged.
From: ros-dev-bounces@reactos.org [mailto:ros-dev-bounces@reactos.org] On Behalf Of Aleksey Bragin
Sent: 04 January 2013 14:15
To: ReactOS Development List
Subject: Re: [ros-dev] [ros-diffs] [hbelusca] 58110: while (TRUE); (when something is unimplemented) ---> ASSERT(FALSE); // while (TRUE); (unless we deal with a 'noreturn' function), and in some cases, return an adequate value. Part...
this while (TRUE); means that "OS can't function anymore if this codepath is executed". It's a fatal, terminal way, so there is no power to save :)
On 04.01.2013 18:11, Javier Agustėn Fernāndez Arroyo wrote:
well, IMO, "while(true)" means CPU is permanently busy, while an ASSERT stops CPU from running ....
just a matter of power saving....
and i repeat, its just an opinion....On Fri, Jan 4, 2013 at 3:02 PM, Aleksey Bragin <aleksey@reactos.org> wrote:
With all respect, I don't understand many of these changes. Answering between the lines.
On 04.01.2013 15:47, hbelusca@svn.reactos.org wrote:NTSTATUS
@@ -643,7 +643,8 @@
/* FIXME: TODO */
DPRINT1("You have implemented the KD routines for searching PCI debugger"
"devices, but you have forgotten to implement this routine\n");
- while (TRUE);
+ UNIMPLEMENTED;
+ ASSERT(FALSE); // while (TRUE);
}It already prints a mandatory log message that this part is unimplemented. And execution is supposed to stop after printing this message, because it's meaningless to continue (that's why while(TRUE); was put there in the first place).
static ULONG NTAPI
@@ -678,7 +679,7 @@
{
/* /PCILOCK is not yet supported */
UNIMPLEMENTED;
- while (TRUE);
+ ASSERT(FALSE); // while (TRUE);
}
#endif
/* Now create the correct resource list based on the supported bus ranges */It already has UNIMPLEMENTED; and now you added an ASSERT(FALSE); which essentially is the same thing. And if continuation is possible, then just UNIMPLEMENTED would be enough.
I'd like some general solution to this. Like, UNIMPLEMENTED_FATAL() or something like that.
Any thoughts?
Regards,
Aleksey Bragin