* [PATCH v2 1/3] tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment
2020-02-17 9:42 [PATCH v2 0/3] tools/virtiofsd: Fix warning reported by Clang static code analyzer Philippe Mathieu-Daudé
@ 2020-02-17 9:42 ` Philippe Mathieu-Daudé
2020-02-17 11:37 ` Dr. David Alan Gilbert
2020-02-17 9:42 ` [PATCH v2 2/3] " Philippe Mathieu-Daudé
2020-02-17 9:42 ` [RFC PATCH v2 3/3] tools/virtiofsd/fuse_lowlevel: Fix fuse_out_header::error value Philippe Mathieu-Daudé
2 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-17 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé,
Ján Tomko, Dr. David Alan Gilbert, Stefan Hajnoczi
Fix warning reported by Clang static code analyzer:
CC tools/virtiofsd/passthrough_ll.o
tools/virtiofsd/passthrough_ll.c:1083:5: warning: Value stored to 'saverr' is never read
saverr = ENOMEM;
^ ~~~~~~
Fixes: 7c6b66027
Reported-by: Clang Static Analyzer
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
tools/virtiofsd/passthrough_ll.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index c635fc8820..e9e71d5fc2 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -1080,8 +1080,6 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t parent,
return;
}
- saverr = ENOMEM;
-
saverr = lo_change_cred(req, &old);
if (saverr) {
goto out;
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/3] tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment
2020-02-17 9:42 ` [PATCH v2 1/3] tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment Philippe Mathieu-Daudé
@ 2020-02-17 11:37 ` Dr. David Alan Gilbert
2020-02-17 14:12 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-17 11:37 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Ján Tomko, qemu-devel, Stefan Hajnoczi
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Fix warning reported by Clang static code analyzer:
>
> CC tools/virtiofsd/passthrough_ll.o
> tools/virtiofsd/passthrough_ll.c:1083:5: warning: Value stored to 'saverr' is never read
> saverr = ENOMEM;
> ^ ~~~~~~
>
> Fixes: 7c6b66027
> Reported-by: Clang Static Analyzer
> Reviewed-by: Ján Tomko <jtomko@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Thanks, you might want to snd that to the upstream libfuse
(different context); it looks like it's the same there.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Dave
> ---
> tools/virtiofsd/passthrough_ll.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index c635fc8820..e9e71d5fc2 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -1080,8 +1080,6 @@ static void lo_mknod_symlink(fuse_req_t req, fuse_ino_t parent,
> return;
> }
>
> - saverr = ENOMEM;
> -
> saverr = lo_change_cred(req, &old);
> if (saverr) {
> goto out;
> --
> 2.21.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v2 1/3] tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment
2020-02-17 11:37 ` Dr. David Alan Gilbert
@ 2020-02-17 14:12 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-17 14:12 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Ján Tomko, qemu-devel, Stefan Hajnoczi
On 2/17/20 12:37 PM, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
>> Fix warning reported by Clang static code analyzer:
>>
>> CC tools/virtiofsd/passthrough_ll.o
>> tools/virtiofsd/passthrough_ll.c:1083:5: warning: Value stored to 'saverr' is never read
>> saverr = ENOMEM;
>> ^ ~~~~~~
>>
>> Fixes: 7c6b66027
>> Reported-by: Clang Static Analyzer
>> Reviewed-by: Ján Tomko <jtomko@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Thanks, you might want to snd that to the upstream libfuse
> (different context); it looks like it's the same there.
Done: https://github.com/libfuse/libfuse/pull/501
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment
2020-02-17 9:42 [PATCH v2 0/3] tools/virtiofsd: Fix warning reported by Clang static code analyzer Philippe Mathieu-Daudé
2020-02-17 9:42 ` [PATCH v2 1/3] tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment Philippe Mathieu-Daudé
@ 2020-02-17 9:42 ` Philippe Mathieu-Daudé
2020-02-17 11:42 ` Ján Tomko
2020-02-17 12:07 ` Dr. David Alan Gilbert
2020-02-17 9:42 ` [RFC PATCH v2 3/3] tools/virtiofsd/fuse_lowlevel: Fix fuse_out_header::error value Philippe Mathieu-Daudé
2 siblings, 2 replies; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-17 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé,
Ján Tomko, Dr. David Alan Gilbert, Stefan Hajnoczi
Fix warning reported by Clang static code analyzer:
CC tools/virtiofsd/passthrough_ll.o
tools/virtiofsd/passthrough_ll.c:925:9: warning: Value stored to 'newfd' is never read
newfd = -1;
^ ~~
tools/virtiofsd/passthrough_ll.c:942:9: warning: Value stored to 'newfd' is never read
newfd = -1;
^ ~~
Fixes: 7c6b66027
Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
v2: do not set newfd, use it (jtomko)
---
tools/virtiofsd/passthrough_ll.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index e9e71d5fc2..02ff01fad0 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -922,7 +922,6 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
inode = lo_find(lo, &e->attr);
if (inode) {
close(newfd);
- newfd = -1;
} else {
inode = calloc(1, sizeof(struct lo_inode));
if (!inode) {
@@ -939,7 +938,6 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
inode->nlookup = 1;
inode->fd = newfd;
- newfd = -1;
inode->key.ino = e->attr.st_ino;
inode->key.dev = e->attr.st_dev;
pthread_mutex_init(&inode->plock_mutex, NULL);
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v2 2/3] tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment
2020-02-17 9:42 ` [PATCH v2 2/3] " Philippe Mathieu-Daudé
@ 2020-02-17 11:42 ` Ján Tomko
2020-02-17 12:07 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 11+ messages in thread
From: Ján Tomko @ 2020-02-17 11:42 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Stefan Hajnoczi, Dr. David Alan Gilbert
[-- Attachment #1: Type: text/plain, Size: 758 bytes --]
On Mon, Feb 17, 2020 at 10:42:39AM +0100, Philippe Mathieu-Daudé wrote:
>Fix warning reported by Clang static code analyzer:
>
> CC tools/virtiofsd/passthrough_ll.o
> tools/virtiofsd/passthrough_ll.c:925:9: warning: Value stored to 'newfd' is never read
> newfd = -1;
> ^ ~~
> tools/virtiofsd/passthrough_ll.c:942:9: warning: Value stored to 'newfd' is never read
> newfd = -1;
> ^ ~~
>
>Fixes: 7c6b66027
>Reported-by: Clang Static Analyzer
>Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>---
>v2: do not set newfd, use it (jtomko)
>---
> tools/virtiofsd/passthrough_ll.c | 2 --
> 1 file changed, 2 deletions(-)
>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2 2/3] tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment
2020-02-17 9:42 ` [PATCH v2 2/3] " Philippe Mathieu-Daudé
2020-02-17 11:42 ` Ján Tomko
@ 2020-02-17 12:07 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-17 12:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Ján Tomko, qemu-devel, Stefan Hajnoczi
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Fix warning reported by Clang static code analyzer:
>
> CC tools/virtiofsd/passthrough_ll.o
> tools/virtiofsd/passthrough_ll.c:925:9: warning: Value stored to 'newfd' is never read
> newfd = -1;
> ^ ~~
> tools/virtiofsd/passthrough_ll.c:942:9: warning: Value stored to 'newfd' is never read
> newfd = -1;
> ^ ~~
>
> Fixes: 7c6b66027
> Reported-by: Clang Static Analyzer
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
OK; actually one of those is in upstream, and another was added
by Miklos's 'virtiofsd: fail when parent inode isn't known in
lo_do_lookup()' - which kind of balances that if out.
I suspect the reason was because out_err: closes newfd if it's
not -1, so they were keeping it safe like that just in case another
error case gets added.
Dave
> ---
> v2: do not set newfd, use it (jtomko)
> ---
> tools/virtiofsd/passthrough_ll.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index e9e71d5fc2..02ff01fad0 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -922,7 +922,6 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
> inode = lo_find(lo, &e->attr);
> if (inode) {
> close(newfd);
> - newfd = -1;
> } else {
> inode = calloc(1, sizeof(struct lo_inode));
> if (!inode) {
> @@ -939,7 +938,6 @@ static int lo_do_lookup(fuse_req_t req, fuse_ino_t parent, const char *name,
>
> inode->nlookup = 1;
> inode->fd = newfd;
> - newfd = -1;
> inode->key.ino = e->attr.st_ino;
> inode->key.dev = e->attr.st_dev;
> pthread_mutex_init(&inode->plock_mutex, NULL);
> --
> 2.21.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread
* [RFC PATCH v2 3/3] tools/virtiofsd/fuse_lowlevel: Fix fuse_out_header::error value
2020-02-17 9:42 [PATCH v2 0/3] tools/virtiofsd: Fix warning reported by Clang static code analyzer Philippe Mathieu-Daudé
2020-02-17 9:42 ` [PATCH v2 1/3] tools/virtiofsd/passthrough_ll: Remove unneeded variable assignment Philippe Mathieu-Daudé
2020-02-17 9:42 ` [PATCH v2 2/3] " Philippe Mathieu-Daudé
@ 2020-02-17 9:42 ` Philippe Mathieu-Daudé
2020-02-17 20:06 ` Dr. David Alan Gilbert
2 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-17 9:42 UTC (permalink / raw)
To: qemu-devel
Cc: Philippe Mathieu-Daudé,
Ján Tomko, Dr. David Alan Gilbert, Stefan Hajnoczi
Fix warning reported by Clang static code analyzer:
CC tools/virtiofsd/fuse_lowlevel.o
tools/virtiofsd/fuse_lowlevel.c:195:9: warning: Value stored to 'error' is never read
error = -ERANGE;
^ ~~~~~~~
Fixes: 2de121f01e
Reported-by: Clang Static Analyzer
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
RFC because untested
---
tools/virtiofsd/fuse_lowlevel.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
index 704c0369b2..2dd36ec03b 100644
--- a/tools/virtiofsd/fuse_lowlevel.c
+++ b/tools/virtiofsd/fuse_lowlevel.c
@@ -192,7 +192,7 @@ int fuse_send_reply_iov_nofree(fuse_req_t req, int error, struct iovec *iov,
if (error <= -1000 || error > 0) {
fuse_log(FUSE_LOG_ERR, "fuse: bad error value: %i\n", error);
- error = -ERANGE;
+ out.error = -ERANGE;
}
iov[0].iov_base = &out;
--
2.21.1
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [RFC PATCH v2 3/3] tools/virtiofsd/fuse_lowlevel: Fix fuse_out_header::error value
2020-02-17 9:42 ` [RFC PATCH v2 3/3] tools/virtiofsd/fuse_lowlevel: Fix fuse_out_header::error value Philippe Mathieu-Daudé
@ 2020-02-17 20:06 ` Dr. David Alan Gilbert
2020-02-18 6:07 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-17 20:06 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Ján Tomko, qemu-devel, Stefan Hajnoczi
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> Fix warning reported by Clang static code analyzer:
>
> CC tools/virtiofsd/fuse_lowlevel.o
> tools/virtiofsd/fuse_lowlevel.c:195:9: warning: Value stored to 'error' is never read
> error = -ERANGE;
> ^ ~~~~~~~
>
> Fixes: 2de121f01e
> Reported-by: Clang Static Analyzer
> Reviewed-by: Ján Tomko <jtomko@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Nice, this actually fixes 3db2876
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> RFC because untested
> ---
> tools/virtiofsd/fuse_lowlevel.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> index 704c0369b2..2dd36ec03b 100644
> --- a/tools/virtiofsd/fuse_lowlevel.c
> +++ b/tools/virtiofsd/fuse_lowlevel.c
> @@ -192,7 +192,7 @@ int fuse_send_reply_iov_nofree(fuse_req_t req, int error, struct iovec *iov,
>
> if (error <= -1000 || error > 0) {
> fuse_log(FUSE_LOG_ERR, "fuse: bad error value: %i\n", error);
> - error = -ERANGE;
> + out.error = -ERANGE;
> }
>
> iov[0].iov_base = &out;
> --
> 2.21.1
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC PATCH v2 3/3] tools/virtiofsd/fuse_lowlevel: Fix fuse_out_header::error value
2020-02-17 20:06 ` Dr. David Alan Gilbert
@ 2020-02-18 6:07 ` Philippe Mathieu-Daudé
2020-02-18 9:51 ` Dr. David Alan Gilbert
0 siblings, 1 reply; 11+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-02-18 6:07 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Ján Tomko, qemu-devel, Stefan Hajnoczi
On 2/17/20 9:06 PM, Dr. David Alan Gilbert wrote:
> * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
>> Fix warning reported by Clang static code analyzer:
>>
>> CC tools/virtiofsd/fuse_lowlevel.o
>> tools/virtiofsd/fuse_lowlevel.c:195:9: warning: Value stored to 'error' is never read
>> error = -ERANGE;
>> ^ ~~~~~~~
>>
>> Fixes: 2de121f01e
>> Reported-by: Clang Static Analyzer
>> Reviewed-by: Ján Tomko <jtomko@redhat.com>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Nice, this actually fixes 3db2876
Good catch, so this fix can be applied to QEMU (while patch #1 needs
backport from upstream libfuse), right?
>
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Thanks.
>
>> ---
>> RFC because untested
>> ---
>> tools/virtiofsd/fuse_lowlevel.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
>> index 704c0369b2..2dd36ec03b 100644
>> --- a/tools/virtiofsd/fuse_lowlevel.c
>> +++ b/tools/virtiofsd/fuse_lowlevel.c
>> @@ -192,7 +192,7 @@ int fuse_send_reply_iov_nofree(fuse_req_t req, int error, struct iovec *iov,
>>
>> if (error <= -1000 || error > 0) {
>> fuse_log(FUSE_LOG_ERR, "fuse: bad error value: %i\n", error);
>> - error = -ERANGE;
>> + out.error = -ERANGE;
>> }
>>
>> iov[0].iov_base = &out;
>> --
>> 2.21.1
>>
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [RFC PATCH v2 3/3] tools/virtiofsd/fuse_lowlevel: Fix fuse_out_header::error value
2020-02-18 6:07 ` Philippe Mathieu-Daudé
@ 2020-02-18 9:51 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 11+ messages in thread
From: Dr. David Alan Gilbert @ 2020-02-18 9:51 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Ján Tomko, qemu-devel, Stefan Hajnoczi
* Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> On 2/17/20 9:06 PM, Dr. David Alan Gilbert wrote:
> > * Philippe Mathieu-Daudé (philmd@redhat.com) wrote:
> > > Fix warning reported by Clang static code analyzer:
> > >
> > > CC tools/virtiofsd/fuse_lowlevel.o
> > > tools/virtiofsd/fuse_lowlevel.c:195:9: warning: Value stored to 'error' is never read
> > > error = -ERANGE;
> > > ^ ~~~~~~~
> > >
> > > Fixes: 2de121f01e
> > > Reported-by: Clang Static Analyzer
> > > Reviewed-by: Ján Tomko <jtomko@redhat.com>
> > > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >
> > Nice, this actually fixes 3db2876
>
> Good catch, so this fix can be applied to QEMU (while patch #1 needs
> backport from upstream libfuse), right?
I'll take all 3 into QEMU, libfuse can take it asynchronously.
Dave
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> Thanks.
>
> >
> > > ---
> > > RFC because untested
> > > ---
> > > tools/virtiofsd/fuse_lowlevel.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/virtiofsd/fuse_lowlevel.c b/tools/virtiofsd/fuse_lowlevel.c
> > > index 704c0369b2..2dd36ec03b 100644
> > > --- a/tools/virtiofsd/fuse_lowlevel.c
> > > +++ b/tools/virtiofsd/fuse_lowlevel.c
> > > @@ -192,7 +192,7 @@ int fuse_send_reply_iov_nofree(fuse_req_t req, int error, struct iovec *iov,
> > > if (error <= -1000 || error > 0) {
> > > fuse_log(FUSE_LOG_ERR, "fuse: bad error value: %i\n", error);
> > > - error = -ERANGE;
> > > + out.error = -ERANGE;
> > > }
> > > iov[0].iov_base = &out;
> > > --
> > > 2.21.1
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> >
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 11+ messages in thread