Btrfs devel - Sep 2012 - [PATCH] Btrfs: fix return value check in btrfs_ioctl_send()

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

In case of error, the function btrfs_read_fs_root_no_name() returns
ERR_PTR() and never returns NULL pointer. The NULL test in the return
value check should be replaced with IS_ERR(), and remove useless
NULL test.

dpatch engine is used to auto generated this patch.
(https://github.com/weiyj/dpatch)

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 fs/btrfs/send.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index fb5ffe9..a617451 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -4501,10 +4501,6 @@ long btrfs_ioctl_send(struct file *mnt_file, void __user
*arg_)
 			key.type = BTRFS_ROOT_ITEM_KEY;
 			key.offset = (u64)-1;
 			clone_root = btrfs_read_fs_root_no_name(fs_info, &key);
-			if (!clone_root) {
-				ret = -EINVAL;
-				goto out;
-			}
 			if (IS_ERR(clone_root)) {
 				ret = PTR_ERR(clone_root);
 				goto out;
@@ -4520,8 +4516,8 @@ long btrfs_ioctl_send(struct file *mnt_file, void __user
*arg_)
 		key.type = BTRFS_ROOT_ITEM_KEY;
 		key.offset = (u64)-1;
 		sctx->parent_root = btrfs_read_fs_root_no_name(fs_info, &key);
-		if (!sctx->parent_root) {
-			ret = -EINVAL;
+		if (IS_ERR(sctx->parent_root)) {
+			ret = PTR_ERR(sctx->parent_root);
 			goto out;
 		}
 	}


--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Fri, Sep 21, 2012 at 12:42:02PM +0800, Wei Yongjun
wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> 
> In case of error, the function btrfs_read_fs_root_no_name() returns
> ERR_PTR() and never returns NULL pointer. The NULL test in the return
> value check should be replaced with IS_ERR(), and remove useless
> NULL test.
Good catch.  Also please update the other remaining case in
disk-io.c:open_ctree()

2587         fs_info->fs_root = btrfs_read_fs_root_no_name(fs_info,
&location);
2588         if (!fs_info->fs_root)
2589                 goto fail_qgroup;
2590         if (IS_ERR(fs_info->fs_root)) {
2591                 err = PTR_ERR(fs_info->fs_root);
2592                 goto fail_qgroup;
2593         }

> dpatch engine is used to auto generated this patch.
> (https://github.com/weiyj/dpatch)
I''ve played with it, looks nice, although it''s full of
hardcoded paths.
I''d like to run it directly from the git repo. Can you please fix that?
No patch from me, because I hadcoded my paths :)

It would be great if we can run a set of .cocci scripts that would
verify code invariants (limited to cocci capabilities, but eg. the
IS_ERR is a good example) and add new ones eventually over time.


david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 09/24/2012 08:42 PM, David Sterba wrote:
> On Fri, Sep 21, 2012 at 12:42:02PM +0800, Wei Yongjun wrote:
>> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>>
>> In case of error, the function btrfs_read_fs_root_no_name() returns
>> ERR_PTR() and never returns NULL pointer. The NULL test in the return
>> value check should be replaced with IS_ERR(), and remove useless
>> NULL test.
> Good catch.  Also please update the other remaining case in
> disk-io.c:open_ctree()
>
> 2587         fs_info->fs_root = btrfs_read_fs_root_no_name(fs_info,
&location);
> 2588         if (!fs_info->fs_root)
> 2589                 goto fail_qgroup;
> 2590         if (IS_ERR(fs_info->fs_root)) {
> 2591                 err = PTR_ERR(fs_info->fs_root);
> 2592                 goto fail_qgroup;
> 2593         }
>
>
will do in the patch v2.
>> dpatch engine is used to auto generated this patch.
>> (https://github.com/weiyj/dpatch)
> I''ve played with it, looks nice, although it''s full of
hardcoded paths.
Yes, it is current hardcoded path, because I used the linux.git and
linux-next.git, and dpatch will update the git tree every day.

After login, you can change the git url in the web.
> I''d like to run it directly from the git repo. Can you please fix
that?
> No patch from me, because I hadcoded my paths :)
Do you mean does not need auto update git repo, only scan the
change under your git repo?
>
> It would be great if we can run a set of .cocci scripts that would
> verify code invariants (limited to cocci capabilities, but eg. the
> IS_ERR is a good example) and add new ones eventually over time.
By default, It does not include cocci scripts, only include dup include,
useless ''linux/version.h'' check engine, and is disabled.

I am now testing some cocci scripts, and will add them to the github,
so, everyone can import those cocci scripts to dpatch, also they can
add the cocci scripts create by themself.
^_^

Thanks very much for you advise.

Regards,
Yongjun Wei



--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Mon, Sep 24, 2012 at 09:40:00PM +0800, Wei Yongjun
wrote:
> >> dpatch engine is used to auto generated this patch.
> >> (https://github.com/weiyj/dpatch)
> > I''ve played with it, looks nice, although it''s full
of hardcoded paths.
> 
> Yes, it is current hardcoded path, because I used the linux.git and
> linux-next.git, and dpatch will update the git tree every day.
> After login, you can change the git url in the web.
Sure, I was able to point the git source to my local linux.git.
> > I''d like to run it directly from the git repo. Can you please
fix that?
> > No patch from me, because I hadcoded my paths :)
> 
> Do you mean does not need auto update git repo, only scan the
> change under your git repo?
I''m talking about hardcoded paths for the scripts in bin/ like

dpatch/views/engine.py:
args = ''/usr/dpatch/bin/importcocci.sh %s'' % fname

or dpatch/models.py:
return ''/var/lib/dpatch/repo/'' + dname

Not to say that ''/usr/'' is not the right place for adding new
per-package subdirs, refer to
http://www.pathname.com/fhs/pub/fhs-2.3.html#THEUSRHIERARCHY
.

(Also the database has hardcoded path into /var/lib/dpatch)
> > It would be great if we can run a set of .cocci scripts that would
> > verify code invariants (limited to cocci capabilities, but eg. the
> > IS_ERR is a good example) and add new ones eventually over time.
> 
> By default, It does not include cocci scripts, only include dup include,
> useless ''linux/version.h'' check engine, and is disabled.
No problem that the .cocci scripts are not there, I''m fine with adding
them manually now, just to test the tool, but all the hardcoded paths
didn''t let me do that :)

david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On 09/24/2012 10:03 PM, David Sterba wrote:
> On Mon, Sep 24, 2012 at 09:40:00PM +0800, Wei Yongjun wrote:
>>>> dpatch engine is used to auto generated this patch.
>>>> (https://github.com/weiyj/dpatch)
>>> I''ve played with it, looks nice, although it''s
full of hardcoded paths.
>> Yes, it is current hardcoded path, because I used the linux.git and
>> linux-next.git, and dpatch will update the git tree every day.
>> After login, you can change the git url in the web.
> Sure, I was able to point the git source to my local linux.git.
>
>>> I''d like to run it directly from the git repo. Can you
please fix that?
>>> No patch from me, because I hadcoded my paths :)
>> Do you mean does not need auto update git repo, only scan the
>> change under your git repo?
> I''m talking about hardcoded paths for the scripts in bin/ like
>
> dpatch/views/engine.py:
> args = ''/usr/dpatch/bin/importcocci.sh %s'' % fname
>
> or dpatch/models.py:
> return ''/var/lib/dpatch/repo/'' + dname
>
> Not to say that ''/usr/'' is not the right place for adding
new
> per-package subdirs, refer to
> http://www.pathname.com/fhs/pub/fhs-2.3.html#THEUSRHIERARCHY
I have fix the hardcode path issue, and allow dpatch
to be run without installation.
> .
>
> (Also the database has hardcoded path into /var/lib/dpatch)
>
>>> It would be great if we can run a set of .cocci scripts that would
>>> verify code invariants (limited to cocci capabilities, but eg. the
>>> IS_ERR is a good example) and add new ones eventually over time.
>> By default, It does not include cocci scripts, only include dup
include,
>> useless ''linux/version.h'' check engine, and is
disabled.
> No problem that the .cocci scripts are not there, I''m fine with
adding
> them manually now, just to test the tool, but all the hardcoded paths
> didn''t let me do that :)
I have put a sample cocci script to pattern/cocci dir, and we can
import other cocci file using command:

./bin/importcocci.sh /path/to/file.cocci

and then enable it vi the web ui.

the import file have special format:

/// title for patch
///
/// patch description
///
cocci script content
...

and then your can manual scan using the following command:

$ ./bin/dailypatch.sh      patch scan + build test

or
$./bin/dailyupdate.sh     patch scan

or
$ ./bin/dailybuild.sh    build test


The database is adminstrator is admin, passwd: 111111

Regards,
Yongjun Wei

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html