Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-08 Thread Markus Armbruster
malc av1...@comtv.ru writes: On Mon, 7 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Mon, 30 Nov 2009, Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends. Revert that, but take care never

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-08 Thread Markus Armbruster
Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends. Revert that, but take care never to return a null pointer, like malloc() friends may do (it's implementation defined),

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-08 Thread Kevin Wolf
Am 07.12.2009 18:09, schrieb malc: On Mon, 7 Dec 2009, Anthony Liguori wrote: malc wrote: Does anyone object to this moving forward? Yeah, i object to the split production/development qemu_malloc[z]. It's clear to me that there are still improper callers of qemu_malloc() in

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-08 Thread malc
On Tue, 8 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: [..snip..] glibc frees since 1999. From glibc/ChangeLog.10: 1999-04-28 Andreas Jaeger a...@arthur.rhein-neckar.de * malloc/malloc.c (REALLOC_ZERO_BYTES_FREES): Define it to follow ISO C9x and

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Kevin Wolf
Am 06.12.2009 13:00, schrieb malc: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: [..snip..] read(fd, malloc(0), 0) is just fine, because read() doesn't touch the buffer when the

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Kevin Wolf
Am 05.12.2009 18:27, schrieb Anthony Liguori: If qemu_malloc() didn't carry the name malloc() then semantics with size=0 would be a different discussion. But so far, all qemu_* functions tend to behave almost exactly like their C counterparts. Relying on the result of size=0 with malloc()

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Ian Molton
Jamie Lokier wrote: If the system as a whole runs out of memory so that no-overcommit malloc() fails on a small alloc, there's a good chance that you won't be able to send a message to the host Send what message to the host? If the malloc in the socet reconnect code fails, its the code on the

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Markus Armbruster
malc av1...@comtv.ru writes: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: [..snip..] read(fd, malloc(0), 0) is just fine, because read() doesn't touch the buffer when

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Markus Armbruster
Ian Molton ian.mol...@collabora.co.uk writes: Markus Armbruster wrote: p = malloc(n * sizeof(struct foo); if (n !p) exit_no_mem(); for (i = 0; i n; i++) compute_one(p, i); With qemu_malloc(), the error handling moves into qemu_malloc(): p =

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/06/2009 08:41 PM, malc wrote: Sure. My point is that sometimes failure to allocate is due to bugs, invalid input etc, and conversion to OOM aborts en masse, might have not been the best possible route to take, but most likely it was better than doing nothing. I agree. Early oom

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Markus Armbruster
Jamie Lokier ja...@shareable.org writes: Ian Molton wrote: Read the beginning of the thread. Basically it's for arrays, malloc(n * sizeof(x)). well, make sure n is not 0. Its not that hard. I dont think I've *ever* had a situation where I wanted to pass 0 to malloc. I would like to

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread malc
On Mon, 7 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: [..snip..] read(fd, malloc(0), 0)

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Kevin Wolf
Am 07.12.2009 11:00, schrieb malc: Misunderstanding? Such behavior is indeed permissible, and I can't see where I restricted it away. An implementation that behaves as you describe returns pointer to allocated space. That the pointer has some funny bit set doesn't matter. That it can't be

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Kevin Wolf
Am 07.12.2009 10:47, schrieb Avi Kivity: On 12/06/2009 08:41 PM, malc wrote: Sure. My point is that sometimes failure to allocate is due to bugs, invalid input etc, and conversion to OOM aborts en masse, might have not been the best possible route to take, but most likely it was better than

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Markus Armbruster
malc av1...@comtv.ru writes: On Mon, 7 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: [..snip..]

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread malc
On Mon, 30 Nov 2009, Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends. Revert that, but take care never to return a null pointer, like malloc() friends may do (it's implementation defined), because that's another

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Markus Armbruster
malc av1...@comtv.ru writes: On Mon, 30 Nov 2009, Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends. Revert that, but take care never to return a null pointer, like malloc() friends may do (it's implementation

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends. Revert that, but take care never to return a null pointer, like malloc() friends may do (it's implementation defined), because that's another source of bugs. While

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/07/2009 05:50 PM, Anthony Liguori wrote: While it's always fun to argue about standards interpretation, I wanted to capture some action items from the discussion that I think there is agreement about. Since I want to make changes for 0.12, I think it would be best to try and settle

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Avi Kivity wrote: On 12/07/2009 05:50 PM, Anthony Liguori wrote: While it's always fun to argue about standards interpretation, I wanted to capture some action items from the discussion that I think there is agreement about. Since I want to make changes for 0.12, I think it would be best

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/07/2009 06:06 PM, Anthony Liguori wrote: Avi Kivity wrote: On 12/07/2009 05:50 PM, Anthony Liguori wrote: While it's always fun to argue about standards interpretation, I wanted to capture some action items from the discussion that I think there is agreement about. Since I want to

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Avi Kivity wrote: Covering every qemu_malloc instance this close to the GA is too risky. I agree that having separate behavior is less than ideal but I think it's the only sane way forward. I don't understand why. What's so insane about Markus' patch? Allowing size=0 for both developer

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Paul Brook
type *qemu_new(type, n_types); type *qemu_new0(type, n_types); type *qemu_renew(type, mem, n_types); type *qemu_renew0(type, mem, n_types); It always annoys me having to specify element count for things that aren't arrays. I suggestion a single object allocation function, and an array

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/07/2009 06:20 PM, Anthony Liguori wrote: Avi Kivity wrote: Covering every qemu_malloc instance this close to the GA is too risky. I agree that having separate behavior is less than ideal but I think it's the only sane way forward. I don't understand why. What's so insane about

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Paul Brook wrote: type *qemu_new(type, n_types); type *qemu_new0(type, n_types); type *qemu_renew(type, mem, n_types); type *qemu_renew0(type, mem, n_types); It always annoys me having to specify element count for things that aren't arrays. I suggestion a single object allocation

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/07/2009 06:24 PM, Paul Brook wrote: Note that conversion to object/type based allocation is not always straightforward because inheritance means we don't have the final object type when doing the allocation. Instead of specifying the size, we can specify a constructor function (we

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Avi Kivity wrote: On 12/07/2009 06:20 PM, Anthony Liguori wrote: Avi Kivity wrote: Covering every qemu_malloc instance this close to the GA is too risky. I agree that having separate behavior is less than ideal but I think it's the only sane way forward. I don't understand why. What's

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/07/2009 06:32 PM, Anthony Liguori wrote: If we apply the patch, the callers are no longer incorrect. Since we're winding down development on that tree, I see no reason for the production build to be correct and the development tree to be incorrect. The problem with this whole

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread malc
On Mon, 7 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Mon, 30 Nov 2009, Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends. Revert that, but take care never to return a null pointer,

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Avi Kivity wrote: What about developers that hit the assert? Do they send patches that fix code that works in production just so they can run in developer mode? Sounds like a good way to get developers to help convert from qemu_malloc() to qemu_new*() :-) If it helps, we can do a grep -l

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
malc wrote: Does anyone object to this moving forward? Yeah, i object to the split production/development qemu_malloc[z]. It's clear to me that there are still improper callers of qemu_malloc() in the tree. How do you propose we address this for 0.12? Aborting in a production

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/07/2009 06:59 PM, Anthony Liguori wrote: Avi Kivity wrote: What about developers that hit the assert? Do they send patches that fix code that works in production just so they can run in developer mode? Sounds like a good way to get developers to help convert from qemu_malloc() to

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Avi Kivity wrote: On 12/07/2009 06:59 PM, Anthony Liguori wrote: Avi Kivity wrote: What about developers that hit the assert? Do they send patches that fix code that works in production just so they can run in developer mode? Sounds like a good way to get developers to help convert from

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/07/2009 07:09 PM, Anthony Liguori wrote: Avi Kivity wrote: On 12/07/2009 06:59 PM, Anthony Liguori wrote: Avi Kivity wrote: What about developers that hit the assert? Do they send patches that fix code that works in production just so they can run in developer mode? Sounds like a

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Avi Kivity wrote: I don't understand. People will develop patches for 0.12 for a while as bugs are reported and fixed. What's the exact problem here? Regards, Anthony Liguori

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/07/2009 07:17 PM, Anthony Liguori wrote: Avi Kivity wrote: I don't understand. People will develop patches for 0.12 for a while as bugs are reported and fixed. What's the exact problem here? Bug reported against qemu. Developer develops patch, while testing qemu crashes on

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Glauber Costa
So the only correct way would be to write: array = malloc(size); if (array == NULL size != 0) {   return -ENOMEM; } Of course we can differentiate. A failed malloc will abort, a successful one will not. If you were writing portable code.  But that's not what people write.  You can

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Avi Kivity wrote: On 12/07/2009 07:17 PM, Anthony Liguori wrote: Avi Kivity wrote: I don't understand. People will develop patches for 0.12 for a while as bugs are reported and fixed. What's the exact problem here? Bug reported against qemu. Developer develops patch, while testing qemu

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Blue Swirl
On Mon, Dec 7, 2009 at 5:50 PM, Anthony Liguori anth...@codemonkey.ws wrote: Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends.  Revert that, but take care never to return a null pointer, like malloc() friends may do

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/07/2009 07:40 PM, Anthony Liguori wrote: Avi Kivity wrote: On 12/07/2009 07:17 PM, Anthony Liguori wrote: Avi Kivity wrote: I don't understand. People will develop patches for 0.12 for a while as bugs are reported and fixed. What's the exact problem here? Bug reported against

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Avi Kivity wrote: My problem is with stable-0.12. Consider upstream fixed. 1) Bug reported against qemu-0.12.0. 2) Developer writes patch against master, submits, all is well except for the CODING_STYLE argument it triggers. 3) Developer writes patch against stable-0.12, can't test because

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Avi Kivity
On 12/07/2009 08:59 PM, Anthony Liguori wrote: Avi Kivity wrote: My problem is with stable-0.12. Consider upstream fixed. 1) Bug reported against qemu-0.12.0. 2) Developer writes patch against master, submits, all is well except for the CODING_STYLE argument it triggers. 3) Developer writes

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-07 Thread Anthony Liguori
Avi Kivity wrote: On 12/07/2009 08:59 PM, Anthony Liguori wrote: Avi Kivity wrote: My problem is with stable-0.12. Consider upstream fixed. 1) Bug reported against qemu-0.12.0. 2) Developer writes patch against master, submits, all is well except for the CODING_STYLE argument it triggers.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Markus Armbruster
Anthony Liguori anth...@codemonkey.ws writes: Avi Kivity wrote: On 12/04/2009 06:49 PM, Anthony Liguori wrote: I still believe that it is poor practice to pass size==0 to *malloc(). I think actively discouraging this in qemu is a good thing because it's a broken idiom. Why? Unless we

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Markus Armbruster
Avi Kivity a...@redhat.com writes: A NEW(type) and ARRAY_NEW(type, count) marcros would improve type safety and plug a dormant buffer overflow due to multiplication overflow, yes. Even qemu_calloc() would be an improvement. But having qemu_malloc() not fix the zero length array case which

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Sat, 5 Dec 2009, Markus Armbruster wrote: Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Markus Armbruster
malc av1...@comtv.ru writes: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Sat, 5 Dec 2009, Markus Armbruster wrote: Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error,

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Blue Swirl
On Sun, Dec 6, 2009 at 10:39 AM, malc av1...@comtv.ru wrote: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Sat, 5 Dec 2009, Markus Armbruster wrote: Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster wrote: Commit a7d27b53 made

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Blue Swirl wrote: On Sun, Dec 6, 2009 at 10:39 AM, malc av1...@comtv.ru wrote: On Sun, 6 Dec 2009, Markus Armbruster wrote: [..snip..] $ gcc mall.c $ ./a.out ptr 0x46974060 $ Changing read count to 1: $ ./a.out ptr 0x41ce0070 a.out: read: Bad address Thanks.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: [..snip..] read(fd, malloc(0), 0) is just fine, because read() doesn't touch the buffer when the size is zero. [..snip..] Yet under linux the address is checked even for zero case. I don't know what a

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Avi Kivity
On 12/06/2009 12:22 PM, malc wrote: Here, i believe, you are inventing artificial restrictions on how malloc behaves, i don't see anything that prevents the implementor from setting aside a range of addresses with 31st bit set as an indicator of zero allocations, and then happily giving it to

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Markus Armbruster
malc av1...@comtv.ru writes: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: [..snip..] read(fd, malloc(0), 0) is just fine, because read() doesn't touch the buffer when the size is zero. [..snip..] Yet under linux the address is checked even for zero

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Avi Kivity wrote: On 12/06/2009 12:22 PM, malc wrote: Here, i believe, you are inventing artificial restrictions on how malloc behaves, i don't see anything that prevents the implementor from setting aside a range of addresses with 31st bit set as an indicator of zero

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: On Sun, 6 Dec 2009, Markus Armbruster wrote: malc av1...@comtv.ru writes: [..snip..] read(fd, malloc(0), 0) is just fine, because read() doesn't touch the buffer when the size is zero.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Avi Kivity
On 12/06/2009 01:53 PM, malc wrote: On Sun, 6 Dec 2009, Avi Kivity wrote: On 12/06/2009 12:22 PM, malc wrote: Here, i believe, you are inventing artificial restrictions on how malloc behaves, i don't see anything that prevents the implementor from setting aside a range of addresses

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Avi Kivity wrote: On 12/06/2009 01:53 PM, malc wrote: On Sun, 6 Dec 2009, Avi Kivity wrote: On 12/06/2009 12:22 PM, malc wrote: Here, i believe, you are inventing artificial restrictions on how malloc behaves, i don't see anything that prevents

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Avi Kivity
On 12/06/2009 02:11 PM, malc wrote: The implementation needs to track which addresses it handed out, since it is required that malloc(0) != malloc(0) (unless both are NULL). You haven't read carefully, i said range. So? There will be tracking, it's the same as with

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Avi Kivity
On 12/06/2009 01:25 AM, Ian Molton wrote: Avi Kivity wrote: It's not that it doesn't have a way to report failure, it's that it doesn't fail. Do you prefer functions that fail and report it to functions that don't fail? You have a way of allocating memory that will _never_ fail?

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Ian Molton
Markus Armbruster wrote: p = malloc(n * sizeof(struct foo); if (n !p) exit_no_mem(); for (i = 0; i n; i++) compute_one(p, i); With qemu_malloc(), the error handling moves into qemu_malloc(): p = qemu_malloc(n * sizeof(struct foo); for (i = 0; i

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Ian Molton
Avi Kivity wrote: On 12/06/2009 01:25 AM, Ian Molton wrote: Avi Kivity wrote: It's not that it doesn't have a way to report failure, it's that it doesn't fail. Do you prefer functions that fail and report it to functions that don't fail? You have a way of allocating memory that

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Avi Kivity
On 12/06/2009 06:58 PM, Ian Molton wrote: Avi Kivity wrote: On 12/06/2009 01:25 AM, Ian Molton wrote: Avi Kivity wrote: It's not that it doesn't have a way to report failure, it's that it doesn't fail. Do you prefer functions that fail and report it to functions that

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Avi Kivity
On 12/06/2009 06:52 PM, Ian Molton wrote: Markus Armbruster wrote: p = malloc(n * sizeof(struct foo); if (n !p) exit_no_mem(); for (i = 0; i n; i++) compute_one(p, i); With qemu_malloc(), the error handling moves into qemu_malloc(): p =

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Avi Kivity wrote: On 12/06/2009 06:58 PM, Ian Molton wrote: Avi Kivity wrote: On 12/06/2009 01:25 AM, Ian Molton wrote: Avi Kivity wrote: It's not that it doesn't have a way to report failure, it's that it doesn't fail.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Avi Kivity
On 12/06/2009 07:47 PM, malc wrote: It will never fail on Linux. On other hosts it prevents a broken oom handler from taking the guest down a death spiral. It fails here all the time i'm sorry to say, i have overcommit disabled (mostly because kpdf when doing a text search tends to

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Avi Kivity
On 12/06/2009 07:45 PM, malc wrote: And you lose the ability to fail gracefully... We never had it. Suppose p is allocated in response to an SCSI register write, and we allocate a scatter-gather list. What do you do if you OOM? Uh, please do not generalize. Sorry.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Avi Kivity wrote: On 12/06/2009 07:47 PM, malc wrote: It will never fail on Linux. On other hosts it prevents a broken oom handler from taking the guest down a death spiral. It fails here all the time i'm sorry to say, i have overcommit disabled

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Avi Kivity wrote: On 12/06/2009 07:45 PM, malc wrote: And you lose the ability to fail gracefully... We never had it. Suppose p is allocated in response to an SCSI register write, and we allocate a scatter-gather list. What do you do if

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Avi Kivity
On 12/06/2009 08:09 PM, malc wrote: Well, i don't have a swap... ~$ cat /proc/meminfo MemTotal: 515708 kB MemFree: 3932 kB Buffers: 10120 kB Cached: 365320 kB SwapCached: 0 kB Active: 238120 kB Inactive: 222396 kB SwapTotal: 0 kB

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Avi Kivity
On 12/06/2009 08:12 PM, malc wrote: Init is pretty easy to handle. I'm worried about runtime where you can't report an error to the guest. Real hardware doesn't oom. Here, i do agree, but mostly because most of the users of allocation functions just themselves returned NULL or -1 or

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Avi Kivity wrote: On 12/06/2009 08:09 PM, malc wrote: Well, i don't have a swap... ~$ cat /proc/meminfo MemTotal: 515708 kB MemFree: 3932 kB Buffers: 10120 kB Cached: 365320 kB SwapCached: 0 kB Active:

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Jamie Lokier
Ian Molton wrote: Read the beginning of the thread. Basically it's for arrays, malloc(n * sizeof(x)). well, make sure n is not 0. Its not that hard. I dont think I've *ever* had a situation where I wanted to pass 0 to malloc. I would like to remind everyone that sizeof(x) can be 0 too.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Jamie Lokier
Avi Kivity wrote: A NEW(type) and ARRAY_NEW(type, count) marcros would improve type safety and plug a dormant buffer overflow due to multiplication overflow, yes. Even qemu_calloc() would be an improvement. In my code I regularly use type_alloc(type) and type_free(type, ptr), giving type

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread malc
On Sun, 6 Dec 2009, Avi Kivity wrote: On 12/06/2009 08:12 PM, malc wrote: Init is pretty easy to handle. I'm worried about runtime where you can't report an error to the guest. Real hardware doesn't oom. Here, i do agree, but mostly because most of the users of allocation

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Ian Molton
Avi Kivity wrote: Init is pretty easy to handle. I'm worried about runtime where you can't report an error to the guest. Real hardware doesn't oom. In the case of the socket reconnect code I posted recently, if the allocation failed, it would give up trying to reconnect and inform the user

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Ian Molton
malc wrote: Well, i don't have a swap... Indeed, nor do I (machine has an NFS root. swap on NFS is... not good.)

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-06 Thread Jamie Lokier
Ian Molton wrote: Avi Kivity wrote: Init is pretty easy to handle. I'm worried about runtime where you can't report an error to the guest. Real hardware doesn't oom. In the case of the socket reconnect code I posted recently, if the allocation failed, it would give up trying to

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Markus Armbruster
Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends. Revert that, but take care never to return a null pointer, like malloc() friends may do (it's implementation defined),

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Laurent Desnogues
Hello, probably a stupid remark, but I'll do it anyway :-) Let's assume there are some contexts where doing a memory allocation with a size of 0 is invalid while in some other contexts it is valid. Wouldn't it make sense in that case to have two functions that do memory allocation? Of course

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Avi Kivity
On 12/04/2009 06:49 PM, Anthony Liguori wrote: I still believe that it is poor practice to pass size==0 to *malloc(). I think actively discouraging this in qemu is a good thing because it's a broken idiom. Why? Unless we have a separate array allocator (like C++'s new and new[]), we need

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread malc
On Sat, 5 Dec 2009, Markus Armbruster wrote: Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends. Revert that, but take care never to return a null pointer, like

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Avi Kivity
On 12/05/2009 07:08 PM, malc wrote: What's the impact of such usage? What would improve for users if it were eradicated? For developers? I believe the answer the first two questions is nothing in particular, and the answer to the last one is hassle. But I'd be happy to see *specific*

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Anthony Liguori
Avi Kivity wrote: On 12/04/2009 06:49 PM, Anthony Liguori wrote: I still believe that it is poor practice to pass size==0 to *malloc(). I think actively discouraging this in qemu is a good thing because it's a broken idiom. Why? Unless we have a separate array allocator (like C++'s new

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Blue Swirl
On Sat, Dec 5, 2009 at 5:07 PM, Avi Kivity a...@redhat.com wrote: On 12/04/2009 06:49 PM, Anthony Liguori wrote: I still believe that it is poor practice to pass size==0 to *malloc().  I think actively discouraging this in qemu is a good thing because it's a broken idiom. Why?  Unless we

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Avi Kivity
On 12/05/2009 07:27 PM, Anthony Liguori wrote: Avi Kivity wrote: On 12/04/2009 06:49 PM, Anthony Liguori wrote: I still believe that it is poor practice to pass size==0 to *malloc(). I think actively discouraging this in qemu is a good thing because it's a broken idiom. Why? Unless we

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Avi Kivity
On 12/05/2009 07:28 PM, Blue Swirl wrote: On Sat, Dec 5, 2009 at 5:07 PM, Avi Kivitya...@redhat.com wrote: On 12/04/2009 06:49 PM, Anthony Liguori wrote: I still believe that it is poor practice to pass size==0 to *malloc(). I think actively discouraging this in qemu is a good

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Anthony Liguori
Avi Kivity wrote: A zero-supporting qemu_malloc() is fully compatible with malloc(), we're only restricting the possible returns. So we're not misleading any caller. In fact, taking your argument to the extreme, a malloc implementation would need to This is really the crux of the whole

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Avi Kivity
On 12/05/2009 07:54 PM, Anthony Liguori wrote: Avi Kivity wrote: A zero-supporting qemu_malloc() is fully compatible with malloc(), we're only restricting the possible returns. So we're not misleading any caller. In fact, taking your argument to the extreme, a malloc implementation would

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Laurent Desnogues
On Sat, Dec 5, 2009 at 6:44 PM, Avi Kivity a...@redhat.com wrote: [...] I think Laurent's proposal would work. We even could go so far as rename the current function as qemu_malloc_possibly_broken (and adjust callers mechanically) and introduce two new versions, which handle the zero case in

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Reimar Döffinger
On Sat, Dec 05, 2009 at 08:08:27PM +0300, malc wrote: ret = read (fd, p, 0); if (ret != 0) err (1, read); return 0; } eof read$ ./a.out a.out: read: Bad address Even though that linux's read(2) man page claims [1]: DESCRIPTION read()

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Anthony Liguori
Avi Kivity wrote: When we see a lengthy and error prone idiom we usually provide a wrapper. That wrapper is qemu_malloc(). If you like, don't see it as a fixed malloc(), but as qemu's way of allocating memory which is totally independent from malloc(). We constantly get patches with

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Avi Kivity
On 12/05/2009 10:58 PM, Anthony Liguori wrote: Avi Kivity wrote: When we see a lengthy and error prone idiom we usually provide a wrapper. That wrapper is qemu_malloc(). If you like, don't see it as a fixed malloc(), but as qemu's way of allocating memory which is totally independent from

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Ian Molton
Avi Kivity wrote: Only if you allocate using POSIX malloc(). If you allocate using a function that is defined to return a valid pointer for zero length allocations, you're happy. Wouldnt it be better to, rather than use a qemu_malloc() that is utterly counterintuitive in that it has no way

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Avi Kivity
On 12/06/2009 01:08 AM, Ian Molton wrote: Avi Kivity wrote: Only if you allocate using POSIX malloc(). If you allocate using a function that is defined to return a valid pointer for zero length allocations, you're happy. Wouldnt it be better to, rather than use a qemu_malloc() that

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Ian Molton
Avi Kivity wrote: It's not that it doesn't have a way to report failure, it's that it doesn't fail. Do you prefer functions that fail and report it to functions that don't fail? You have a way of allocating memory that will _never_ fail? Seriously, who does that anyway? why call malloc

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-05 Thread Markus Armbruster
malc av1...@comtv.ru writes: On Sat, 5 Dec 2009, Markus Armbruster wrote: Anthony Liguori anth...@codemonkey.ws writes: Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends. Revert that, but take care never to

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-04 Thread Anthony Liguori
Markus Armbruster wrote: Commit a7d27b53 made zero-sized allocations a fatal error, deviating from ISO C's malloc() friends. Revert that, but take care never to return a null pointer, like malloc() friends may do (it's implementation defined), because that's another source of bugs.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-01 Thread Gerd Hoffmann
diff --git a/qemu-malloc.c b/qemu-malloc.c index 295d185..aeeb78b 100644 --- a/qemu-malloc.c +++ b/qemu-malloc.c @@ -44,22 +44,12 @@ void qemu_free(void *ptr) void *qemu_malloc(size_t size) { -if (!size) { -abort(); -} -return oom_check(malloc(size)); +return

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-01 Thread Gerd Hoffmann
On 12/01/09 13:40, Gerd Hoffmann wrote: + return oom_check(malloc(size ? size : 1)); void *qemu_realloc(void *ptr, size_t size) { + return oom_check(realloc(ptr, size ? size : 1)); qemu_realloc(qemu_malloc(0), 42); should better work correctly ... Oops, scratch that. return malloc(size

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-01 Thread Paul Brook
You might want to have a 'static uint8_t zero_length_malloc[0]' and return that instead of the magic cookie '1'. Makes the code more readable IMHO and you'll also have symbol in gdb when debugging qemu. Having multiple malloc return the same pointer sounds like a really bad idea. Paul

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

2009-12-01 Thread Markus Armbruster
Gerd Hoffmann kra...@redhat.com writes: diff --git a/qemu-malloc.c b/qemu-malloc.c index 295d185..aeeb78b 100644 --- a/qemu-malloc.c +++ b/qemu-malloc.c @@ -44,22 +44,12 @@ void qemu_free(void *ptr) void *qemu_malloc(size_t size) { -if (!size) { -abort(); -} -

  1   2   >