Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
Markus Hitter wrote: Quite obviously, we have very different expectations on what executables should do. I expect code to be as reliable as possible, to be careful when aquiring resources and if a process would ever happen to make my computer spontanuously reboot, I'd throw that binary as far away as possible and warn everybody else not to even touch this thing. If a user program causes your computer to spontaneously reboot, you should throw away your OS instead of that program. -- error compiling committee.c: too many arguments to function
Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
Am 05.01.2008 um 03:47 schrieb Rob Landley: You can disable overcommit and give the system an egregious amount of swap space, but then your pathological case is the system going into swap thrashing la-la land and essentially freezing (advancing at 0.1% of its normal rate, if that, for _hours_) instead of killing some runaway processes (or rebooting) and recovering. Quite obviously, we have very different expectations on what executables should do. I expect code to be as reliable as possible, to be careful when aquiring resources and if a process would ever happen to make my computer spontanuously reboot, I'd throw that binary as far away as possible and warn everybody else not to even touch this thing. One of the major reasons to run emulators is to protect against such sloppy code, btw. Markus - - - - - - - - - - - - - - - - - - - Dipl. Ing. Markus Hitter http://www.jump-ing.de/
Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
On Friday 04 January 2008 19:07:58 Paul Brook wrote: > > On modern operating systems, allocations only return zero when you > > exhaust virtual memory. Returning nonzero doesn't mean you have enough > > memory, because it's given you a redundant copy on write mapping of the > > zero page and will fault in physical pages when you write to 'em, which > > has _no_ return value. Instead, the out of memory killer will shoot your > > program in the head in the middle of it's run > > Decent operating systems allow the system administrator gets to choose how > optimistic memory allocation is. You're describing wildly-optimistic mode, > which is often but not always the default. True, but if you completely disable overcommit then fork() a large process and exec() a small one, you haven't got enough memory even though you're not really _using_ any to do so. You can disable overcommit and give the system an egregious amount of swap space, but then your pathological case is the system going into swap thrashing la-la land and essentially freezing (advancing at 0.1% of its normal rate, if that, for _hours_) instead of killing some runaway processes (or rebooting) and recovering. Not necessarily and improvement, especially if you're the one with the pager. It is alas, not a simple problem to get right. fork() and exec() being separate system calls isn't always an improvement over a combined one. (Espeically since exec() needs a file, not a file handle. You can't re-exec your current process unless you can find and reopen it, you can't exec() from a pipe... And then there's nommu vfork(), always fun...) > Paul Rob -- "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson.
Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
> On modern operating systems, allocations only return zero when you exhaust > virtual memory. Returning nonzero doesn't mean you have enough memory, > because it's given you a redundant copy on write mapping of the zero page > and will fault in physical pages when you write to 'em, which has _no_ > return value. Instead, the out of memory killer will shoot your program in > the head in the middle of it's run Decent operating systems allow the system administrator gets to choose how optimistic memory allocation is. You're describing wildly-optimistic mode, which is often but not always the default. Paul
Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
On Friday 04 January 2008 11:29:27 Markus Hitter wrote: > Am 03.01.2008 um 15:02 schrieb Paul Brook: > > Having to check every return value is extremely tedious and (as > > you've proved) > > easy to miss. > > Checking every return value is a measure for programming reliable code. There's an old tounge-in-cheek programming rule: "Never test for an error condition you don't know how to handle." An example of "ha ha, only serious". Not only is defensive programming a huge source of bloat, but I've had to deal with a lot of programs where what broke _was_ some defensive check, and the code would have worked just fine if they just hadn't been so paranoid. (So the return value of close() is nonzero. What do you _do_ about it? Calling abort() is not necessarily the sane response.) I've got a shell script where bash spits out an error from an EVVVAR="$(thing | thing2)" construct, despite getting the right answer. Why? Because thing2 exits after consuming all its data, and the left half of the pipeline outlives it by a milisecond or so and then errors out when it tries to flush and close stdout (which has already delivered all its data, but it gets an error return from doing this anyway and darn it, it's going to tell you about it!) I think that to shut it up I need to rephrase it as: ENVVAR="$(thing 2>/dev/null | thing2)" but to be honest, I haven't bothered. > > If the allocation fails we don't have any viable alternatives, so > > we may as well stop right there. > > Stop != segfault? What about a meaningful exit message? What if the > code doesn't segfault but runs havok? On modern operating systems, allocations only return zero when you exhaust virtual memory. Returning nonzero doesn't mean you have enough memory, because it's given you a redundant copy on write mapping of the zero page and will fault in physical pages when you write to 'em, which has _no_ return value. Instead, the out of memory killer will shoot your program in the head in the middle of it's run, and your only indication of a real out of memory status is "kill -9". So on a modern system, defensive programming is also somewhat futile. Just FYI. Rob -- "One of my most productive days was throwing away 1000 lines of code." - Ken Thompson.
Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
Paul Brook wrote: What about a meaningful exit message? "Out of memory" is a fairly comprehensive description of the problem. In fact I'd say it's much more informative than "doesn't know or care about> failed to initialize". If the user requested a target parameter that is beyond the capabilities of the host system, a meaningful error message can be generated that instructs the user on how to possibly correct the problem.
Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
On Friday 04 January 2008, Markus Hitter wrote: > Am 03.01.2008 um 15:02 schrieb Paul Brook: > > Having to check every return value is extremely tedious and (as > > you've proved) easy to miss. > > Checking every return value is a measure for programming reliable code. Never failing is even more reliable. > > If the allocation fails we don't have any viable alternatives, so > > we may as well stop right there. > > Stop != segfault? Yes. > What about a meaningful exit message? "Out of memory" is a fairly comprehensive description of the problem. In fact I'd say it's much more informative than " failed to initialize". Paul
Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
Am 03.01.2008 um 15:02 schrieb Paul Brook: Having to check every return value is extremely tedious and (as you've proved) easy to miss. Checking every return value is a measure for programming reliable code. If the allocation fails we don't have any viable alternatives, so we may as well stop right there. Stop != segfault? What about a meaningful exit message? What if the code doesn't segfault but runs havok? Markus - - - - - - - - - - - - - - - - - - - Dipl. Ing. Markus Hitter http://www.jump-ing.de/
Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
> We currently don't check the return value in the init function where the > new timer is created but do check it wherever it is used which is backwards > and wasteful. > > You would prefer that qemu just segfaults rather than die gracefully? I think qemu should die before it returns from qemu_malloc. Having to check every return value is extremely tedious and (as you've proved) easy to miss. If the allocation fails we don't have any viable alternatives, so we may as well stop right there. Paul
Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
Paul Brook wrote: s = (ptimer_state *)qemu_mallocz(sizeof(ptimer_state)); +if (!s) +return NULL; None of the callers bother to check the return value, And even if they did I don't think there's any point trying to gracefully handle OOM. Just abort and be done with it. I am in the process of fixing the sparc ptimer caller to gracefully handle OOM. We currently don't check the return value in the init function where the new timer is created but do check it wherever it is used which is backwards and wasteful. You would prefer that qemu just segfaults rather than die gracefully?
Re: [Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
> s = (ptimer_state *)qemu_mallocz(sizeof(ptimer_state)); > + if (!s) > + return NULL; None of the callers bother to check the return value, And even if they did I don't think there's any point trying to gracefully handle OOM. Just abort and be done with it. I suggest guaranteeing that qemu_malloc will never return NULL, and removing the null checks from all the various users. Paul