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, 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 random widget the user doesn't know or care about 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
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 random widget the user 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 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 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
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
[Qemu-devel] [PATCH] fix possible NULL pointer use in hw/ptimer.c
diff -p -u -r1.5 ptimer.c --- hw/ptimer.c 17 Nov 2007 17:14:47 - 1.5 +++ hw/ptimer.c 3 Jan 2008 02:27:18 - @@ -185,6 +185,8 @@ ptimer_state *ptimer_init(QEMUBH *bh) ptimer_state *s; s = (ptimer_state *)qemu_mallocz(sizeof(ptimer_state)); +if (!s) +return NULL; s-bh = bh; s-timer = qemu_new_timer(vm_clock, ptimer_tick, s); return s;
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
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?
