Re: [Qemu-devel] [PATCH] compatfd.c: Don't pass NULL pointer to SYS_signalfd

2011-10-21 Thread andrzej zaborowski 
On 13 October 2011 19:45, Peter Maydell peter.mayd...@linaro.org wrote:
 Don't pass a NULL pointer in to SYS_signalfd in qemu_signalfd_available():
 this isn't valid and Valgrind complains about it.

Also pushed this patch.

Cheers

Re: [Qemu-devel] [PATCH] compatfd.c: Don't pass NULL pointer to SYS_signalfd

2011-10-14 Thread Stefan Hajnoczi 
On Thu, Oct 13, 2011 at 06:45:37PM +0100, Peter Maydell wrote:
 Don't pass a NULL pointer in to SYS_signalfd in qemu_signalfd_available():
 this isn't valid and Valgrind complains about it.
 
 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
  compatfd.c |   12 ++--
  1 files changed, 10 insertions(+), 2 deletions(-)

Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com

Re: [Qemu-devel] [PATCH] compatfd.c: Don't pass NULL pointer to SYS_signalfd

2011-10-14 Thread Stefan Weil 

Am 13.10.2011 19:45, schrieb Peter Maydell:

Don't pass a NULL pointer in to SYS_signalfd in qemu_signalfd_available():
this isn't valid and Valgrind complains about it.

Signed-off-by: Peter Maydell peter.mayd...@linaro.org
---
compatfd.c | 12 ++--
1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/compatfd.c b/compatfd.c
index 31654c6..02306a4 100644
--- a/compatfd.c
+++ b/compatfd.c
@@ -119,9 +119,17 @@ int qemu_signalfd(const sigset_t *mask)
bool qemu_signalfd_available(void)
{
#ifdef CONFIG_SIGNALFD
+ sigset_t mask;
+ int fd;
+ bool ok;
+ sigemptyset(mask);
errno = 0;
- syscall(SYS_signalfd, -1, NULL, _NSIG / 8);
- return errno != ENOSYS;
+ fd = syscall(SYS_signalfd, -1, mask, _NSIG / 8);
+ ok = (errno != ENOSYS);
+ if (fd = 0) {


Maybe better: fd != -1


+ close(fd);
+ }
+ return ok;
#else
return false;
#endif


The variable 'ok' is not needed, simply returning
errno != ENOSYS would work, too.

Regards,
Stefan W.

Re: [Qemu-devel] [PATCH] compatfd.c: Don't pass NULL pointer to SYS_signalfd

2011-10-14 Thread Peter Maydell 
On 14 October 2011 18:20, Stefan Weil s...@weilnetz.de wrote:
 Am 13.10.2011 19:45, schrieb Peter Maydell:

 Don't pass a NULL pointer in to SYS_signalfd in qemu_signalfd_available():
 this isn't valid and Valgrind complains about it.

 Signed-off-by: Peter Maydell peter.mayd...@linaro.org
 ---
 compatfd.c | 12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

 diff --git a/compatfd.c b/compatfd.c
 index 31654c6..02306a4 100644
 --- a/compatfd.c
 +++ b/compatfd.c
 @@ -119,9 +119,17 @@ int qemu_signalfd(const sigset_t *mask)
 bool qemu_signalfd_available(void)
 {
 #ifdef CONFIG_SIGNALFD
 + sigset_t mask;
 + int fd;
 + bool ok;
 + sigemptyset(mask);
 errno = 0;
 - syscall(SYS_signalfd, -1, NULL, _NSIG / 8);
 - return errno != ENOSYS;
 + fd = syscall(SYS_signalfd, -1, mask, _NSIG / 8);
 + ok = (errno != ENOSYS);
 + if (fd = 0) {

 Maybe better: fd != -1

Style issue -- I prefer the = 0; if you do a
'git grep -A2 open' you'll see that mostly the existing
codebase does 'is it less than zero or not' comparisons
for did this thing returning an fd fail? checks, rather
than 'is it equal to -1 or not'.

 + close(fd);
 + }
 + return ok;
 #else
 return false;
 #endif

 The variable 'ok' is not needed, simply returning
 errno != ENOSYS would work, too.

The call to close() might have trashed errno (although
admittedly the chances of close() returning ENOSYS are
rather low I think it's clearer to return the result
of checking the errno for the syscall we care about rather
than the one we don't).

-- PMM