[patch] fuse: fix an error code in fuse_lookup_name()

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch] fuse: fix an error code in fuse_lookup_name()

Dan Carpenter-2
This is a static checker fix because we check "outarg->nodeid" here and
then again a in the next if statement.  The difference is that for this
first one we don't set the error code.

Fixes: c180eebe1390 ('fuse: add fuse_lookup_name() helper')
Signed-off-by: Dan Carpenter <[hidden email]>

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 5e2e087..225fc703 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -306,8 +306,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, struct qstr *name,
 
  fuse_lookup_init(fc, &args, nodeid, name, outarg);
  err = fuse_simple_request(fc, &args);
- /* Zero nodeid is same as -ENOENT, but with valid timeout */
- if (err || !outarg->nodeid)
+ if (err)
  goto out_put_forget;
 
  err = -EIO;

------------------------------------------------------------------------------
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [patch] fuse: fix an error code in fuse_lookup_name()

Ashish Samant
Reviewed-by: Ashish Samant <[hidden email]>
On 08/08/2015 06:01 AM, Dan Carpenter wrote:

> This is a static checker fix because we check "outarg->nodeid" here and
> then again a in the next if statement.  The difference is that for this
> first one we don't set the error code.
>
> Fixes: c180eebe1390 ('fuse: add fuse_lookup_name() helper')
> Signed-off-by: Dan Carpenter <[hidden email]>
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 5e2e087..225fc703 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -306,8 +306,7 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, struct qstr *name,
>  
>   fuse_lookup_init(fc, &args, nodeid, name, outarg);
>   err = fuse_simple_request(fc, &args);
> - /* Zero nodeid is same as -ENOENT, but with valid timeout */
> - if (err || !outarg->nodeid)
> + if (err)
>   goto out_put_forget;
>  
>   err = -EIO;
>
> ------------------------------------------------------------------------------
> _______________________________________________
> fuse-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/fuse-devel
>


------------------------------------------------------------------------------
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [patch] fuse: fix an error code in fuse_lookup_name()

Miklos Szeredi
> On 08/08/2015 06:01 AM, Dan Carpenter wrote:
>>
>> This is a static checker fix because we check "outarg->nodeid" here and
>> then again a in the next if statement.  The difference is that for this
>> first one we don't set the error code.

Except for the fact that zero error code is correct (as the comment
indicates) and EIO would be wrong.  So the patch actually breaks the
code, not fixes it.

Thanks,
Miklos

>>
>> Fixes: c180eebe1390 ('fuse: add fuse_lookup_name() helper')
>> Signed-off-by: Dan Carpenter <[hidden email]>
>>
>> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
>> index 5e2e087..225fc703 100644
>> --- a/fs/fuse/dir.c
>> +++ b/fs/fuse/dir.c
>> @@ -306,8 +306,7 @@ int fuse_lookup_name(struct super_block *sb, u64
>> nodeid, struct qstr *name,
>>         fuse_lookup_init(fc, &args, nodeid, name, outarg);
>>         err = fuse_simple_request(fc, &args);
>> -       /* Zero nodeid is same as -ENOENT, but with valid timeout */
>> -       if (err || !outarg->nodeid)
>> +       if (err)
>>                 goto out_put_forget;
>>         err = -EIO;
>>
>>
>> ------------------------------------------------------------------------------
>> _______________________________________________
>> fuse-devel mailing list
>> [hidden email]
>> https://lists.sourceforge.net/lists/listinfo/fuse-devel
>>
>

------------------------------------------------------------------------------
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

[patch v2] fuse: remove dead code in fuse_lookup_name()

Dan Carpenter-2
We check "outarg->nodeid" twice in a row and it causes a static checker
warning.  We should return success if "outarg->nodeid" is zero so the
first check is correct and the second one should be deleted.

Signed-off-by: Dan Carpenter <[hidden email]>
---
v2: The first version of this patch introduced a bug into working code.
    (returning error instead of success).  Sorry.  :(

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 5e2e087..56a3463 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -306,11 +306,10 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, struct qstr *name,
 
  fuse_lookup_init(fc, &args, nodeid, name, outarg);
  err = fuse_simple_request(fc, &args);
- /* Zero nodeid is same as -ENOENT, but with valid timeout */
- if (err || !outarg->nodeid)
+ if (err)
  goto out_put_forget;
 
- err = -EIO;
+ /* Zero nodeid is same as -ENOENT, but with valid timeout */
  if (!outarg->nodeid)
  goto out_put_forget;
  if (!fuse_valid_type(outarg->attr.mode))

------------------------------------------------------------------------------
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel
Reply | Threaded
Open this post in threaded view
|

Re: [patch v2] fuse: remove dead code in fuse_lookup_name()

Dan Carpenter-2
Oh...  Crap.  I suck.  Sorry, ignore this patch.  It's buggy as well.

I'm so really really sorry.

regards,
dan carpenter

On Thu, Aug 13, 2015 at 12:06:44AM +0300, Dan Carpenter wrote:

> We check "outarg->nodeid" twice in a row and it causes a static checker
> warning.  We should return success if "outarg->nodeid" is zero so the
> first check is correct and the second one should be deleted.
>
> Signed-off-by: Dan Carpenter <[hidden email]>
> ---
> v2: The first version of this patch introduced a bug into working code.
>     (returning error instead of success).  Sorry.  :(
>
> diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> index 5e2e087..56a3463 100644
> --- a/fs/fuse/dir.c
> +++ b/fs/fuse/dir.c
> @@ -306,11 +306,10 @@ int fuse_lookup_name(struct super_block *sb, u64 nodeid, struct qstr *name,
>  
>   fuse_lookup_init(fc, &args, nodeid, name, outarg);
>   err = fuse_simple_request(fc, &args);
> - /* Zero nodeid is same as -ENOENT, but with valid timeout */
> - if (err || !outarg->nodeid)
> + if (err)
>   goto out_put_forget;
>  
> - err = -EIO;
> + /* Zero nodeid is same as -ENOENT, but with valid timeout */
>   if (!outarg->nodeid)
>   goto out_put_forget;
>   if (!fuse_valid_type(outarg->attr.mode))

------------------------------------------------------------------------------
_______________________________________________
fuse-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/fuse-devel