| Message ID | 1471444382-30409-2-git-send-email-wei.liu2@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 17/08/16 15:33, Wei Liu wrote: > The semantics of alloca(3) is not very nice. If the stack overflows, > program behaviour is undefined. > > Remove the use of alloca(3) and always use mmap. This is only using alloca() if the allocation is < PAGE_SIZE. I think assuming there's this much extra stack is fine. Using alloca() avoids the (expensive) mmap() system call. David
On Mon, Aug 22, 2016 at 10:46:50AM +0100, David Vrabel wrote: > On 17/08/16 15:33, Wei Liu wrote: > > The semantics of alloca(3) is not very nice. If the stack overflows, > > program behaviour is undefined. > > > > Remove the use of alloca(3) and always use mmap. > > This is only using alloca() if the allocation is < PAGE_SIZE. I think > assuming there's this much extra stack is fine. > A library is not in a position assume how deep the stack is IMHO. Wei.
On 22/08/16 11:10, Wei Liu wrote: > On Mon, Aug 22, 2016 at 10:46:50AM +0100, David Vrabel wrote: >> On 17/08/16 15:33, Wei Liu wrote: >>> The semantics of alloca(3) is not very nice. If the stack overflows, >>> program behaviour is undefined. >>> >>> Remove the use of alloca(3) and always use mmap. >> >> This is only using alloca() if the allocation is < PAGE_SIZE. I think >> assuming there's this much extra stack is fine. >> > > A library is not in a position assume how deep the stack is IMHO. This suggests a library cannot use any stack, which is clearly silly. But ok, in which case you should consider using malloc() instead of alloca()/mmap(), then small allocation might come out of some pre-existing or cached allocations. David
On Mon, Aug 22, 2016 at 11:24:56AM +0100, David Vrabel wrote: > On 22/08/16 11:10, Wei Liu wrote: > > On Mon, Aug 22, 2016 at 10:46:50AM +0100, David Vrabel wrote: > >> On 17/08/16 15:33, Wei Liu wrote: > >>> The semantics of alloca(3) is not very nice. If the stack overflows, > >>> program behaviour is undefined. > >>> > >>> Remove the use of alloca(3) and always use mmap. > >> > >> This is only using alloca() if the allocation is < PAGE_SIZE. I think > >> assuming there's this much extra stack is fine. > >> > > > > A library is not in a position assume how deep the stack is IMHO. > > This suggests a library cannot use any stack, which is clearly silly. > Of course not. Please don't take my words out of context and further imply things I never said. This is not how a conversation should work out. And name calling is toxic. Please just stop. I care about the undefined behaviour aspect of alloca -- I believe you dislike that as much as I do. > But ok, in which case you should consider using malloc() instead of > alloca()/mmap(), then small allocation might come out of some > pre-existing or cached allocations. > Ok, that seems sensible. Wei. > David
diff --git a/tools/libs/gnttab/linux.c b/tools/libs/gnttab/linux.c index 7b0fba4..535ce34 100644 --- a/tools/libs/gnttab/linux.c +++ b/tools/libs/gnttab/linux.c @@ -102,18 +102,12 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt, if (flags & XENGNTTAB_GRANT_MAP_SINGLE_DOMAIN) domids_stride = 0; - if ( map_size <= PAGE_SIZE ) - map = alloca(sizeof(*map) + - (count - 1) * sizeof(struct ioctl_gntdev_map_grant_ref)); - else + map = mmap(NULL, map_size, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANON | MAP_POPULATE, -1, 0); + if ( map == MAP_FAILED ) { - map = mmap(NULL, map_size, PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANON | MAP_POPULATE, -1, 0); - if ( map == MAP_FAILED ) - { - GTERROR(xgt->logger, "mmap of map failed"); - return NULL; - } + GTERROR(xgt->logger, "mmap of map failed"); + return NULL; } for ( i = 0; i < count; i++ ) @@ -187,8 +181,7 @@ void *osdep_gnttab_grant_map(xengnttab_handle *xgt, } out: - if ( map_size > PAGE_SIZE ) - munmap(map, map_size); + munmap(map, map_size); return addr; }
The semantics of alloca(3) is not very nice. If the stack overflows, program behaviour is undefined. Remove the use of alloca(3) and always use mmap. Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- tools/libs/gnttab/linux.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-)