|
From: | zhanghailiang |
Subject: | Re: [Qemu-devel] [PATCH 4/4] ivshmem: check the value returned by fstat() |
Date: | Tue, 5 Aug 2014 10:00:41 +0800 |
User-agent: | Mozilla/5.0 (Windows NT 6.1; rv:11.0) Gecko/20120327 Thunderbird/11.0.1 |
Hi Michael, Thanks for your review of this patch!
On Mon, Aug 04, 2014 at 04:25:44PM +0800, zhanghailiang wrote:The function fstat() may fail, so check its return value. Signed-off-by: zhanghailiang<address@hidden> --- hw/misc/ivshmem.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c index 768e528..2667e9f 100644 --- a/hw/misc/ivshmem.c +++ b/hw/misc/ivshmem.c @@ -324,7 +324,10 @@ static int check_shm_size(IVShmemState *s, int fd) { struct stat buf; - fstat(fd,&buf); + if (fstat(fd,&buf)< 0) { + fprintf(stderr, "Cannot stat IVSHMEM: %s\n", strerror(errno)); + return -1; + }That's a confusing error message: 1. You don't stat ivshmem. You stat a shmem fd. Also best to print fd #.
I will add this in next version of the patch.
I have check the places of calling this function, And found that, if this function return -1, qemu will call exit(-1). One of the callers is ivshmem_read(), the purpose of check_shm_size() is to forbid guest to map more memory than the object has allocated. So here is it suitable to return -1 if fstat() failed? Or just give a warning message and return 0?2. Tell the user what action was taken, e.g. IVSHMEM failed to start.if (s->ivshmem_size> buf.st_size) { fprintf(stderr, --
what's your opinion? Thanks.
1.7.12.4.
Best regards, zhanghailiang
[Prev in Thread] | Current Thread | [Next in Thread] |