* [PATCH 1/2] Fix wrong memory free on check_is_root
@ 2014-12-18 6:09 Satoru Takeuchi
2014-12-18 6:42 ` [PATCH 2/2] btrfs-progs: fix the file system root is regarded as non-root Satoru Takeuchi
2014-12-18 7:03 ` [PATCH 1/2] Fix wrong memory free on check_is_root Gui Hecheng
0 siblings, 2 replies; 6+ messages in thread
From: Satoru Takeuchi @ 2014-12-18 6:09 UTC (permalink / raw)
To: linux-btrfs@vger.kernel.org; +Cc: naota
From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
Date: Thu, 18 Dec 2014 14:35:22 +0900
@tmp is freed even if its allocation fails.
Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
---
cmds-property.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/cmds-property.c b/cmds-property.c
index a764293..a4bc127 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -140,31 +140,32 @@ static int check_is_root(const char *object)
if (ret < 0) {
fprintf(stderr, "ERROR: get_fsid for %s failed. %s\n", object,
strerror(-ret));
- goto out;
+ goto free_tmp_out;
}
ret = get_fsid(tmp, fsid2, 1);
if (ret == -ENOTTY) {
ret = 0;
- goto out;
+ goto free_tmp_out;
} else if (ret == -ENOTDIR) {
ret = 1;
- goto out;
+ goto free_tmp_out;
} else if (ret < 0) {
fprintf(stderr, "ERROR: get_fsid for %s failed. %s\n", tmp,
strerror(-ret));
- goto out;
+ goto free_tmp_out;
}
if (memcmp(fsid, fsid2, BTRFS_FSID_SIZE)) {
ret = 0;
- goto out;
+ goto free_tmp_out;
}
ret = 1;
-out:
+free_tmp_out:
free(tmp);
+out:
return ret;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] btrfs-progs: fix the file system root is regarded as non-root
2014-12-18 6:09 [PATCH 1/2] Fix wrong memory free on check_is_root Satoru Takeuchi
@ 2014-12-18 6:42 ` Satoru Takeuchi
2014-12-18 7:09 ` Gui Hecheng
2014-12-18 7:03 ` [PATCH 1/2] Fix wrong memory free on check_is_root Gui Hecheng
1 sibling, 1 reply; 6+ messages in thread
From: Satoru Takeuchi @ 2014-12-18 6:42 UTC (permalink / raw)
To: linux-btrfs@vger.kernel.org; +Cc: naota
From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
When "/" is Btrfs, "btrfs property <subcommand> /" regards it
as non-root by mistake.
check_is_root() regards @object as a file system root if
the following two conditions are satisfied.
a) Both @object and its parent directory are Btrfs object
(file system root, subvolume, inode, and device
used for Btrfs).
b) fsid of the above mentioned two objects are different.
It doesn't work if @object is "/" because, in this case,
fsid of "/" and its parent (it's also "/"), are the same.
* Test environment
Two Btrfs file system (not subvolume) "/" and "/home/sat/mnt".
* How to reproduce
Submit "btrfs prop get" against the above mentioned file systems.
* Test Result
** Actual result (without my patch)
==========================
# btrfs prop get /home/sat/mnt/
ro=false
label= # label is displayed because it's a file system root
# btrfs prop get /
ro=false # label is not displayed even if it's a file system root
==========================
** Expected result (with my patch)
==========================
# ./btrfs-new prop get btrfs-auto-test/
# ./btrfs-new prop get /home/sat/mnt/
ro=false
label=
# ./btrfs-new prop get /
ro=false
label=foo # label is displayed
===========================
Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
Reported-by: Naohiro Aota <naota@elisp.net>
---
cmds-property.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/cmds-property.c b/cmds-property.c
index a4bc127..640af99 100644
--- a/cmds-property.c
+++ b/cmds-property.c
@@ -125,11 +125,22 @@ static int check_is_root(const char *object)
u8 fsid[BTRFS_FSID_SIZE];
u8 fsid2[BTRFS_FSID_SIZE];
char *tmp;
+ char *rp;
+
+ rp = realpath(object, NULL);
+ if (!rp) {
+ ret = -ENOMEM;
+ goto out;
+ }
+ if (!strcmp(rp, "/")) {
+ ret = 0;
+ goto free_rp_out;
+ }
tmp = malloc(strlen(object) + 5);
if (!tmp) {
ret = -ENOMEM;
- goto out;
+ goto free_rp_out;
}
strcpy(tmp, object);
if (tmp[strlen(tmp) - 1] != '/')
@@ -165,6 +176,8 @@ static int check_is_root(const char *object)
free_tmp_out:
free(tmp);
+free_rp_out:
+ free(rp);
out:
return ret;
}
--
1.9.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] btrfs-progs: fix the file system root is regarded as non-root
2014-12-18 6:42 ` [PATCH 2/2] btrfs-progs: fix the file system root is regarded as non-root Satoru Takeuchi
@ 2014-12-18 7:09 ` Gui Hecheng
2014-12-18 7:19 ` Satoru Takeuchi
0 siblings, 1 reply; 6+ messages in thread
From: Gui Hecheng @ 2014-12-18 7:09 UTC (permalink / raw)
To: Satoru Takeuchi; +Cc: linux-btrfs@vger.kernel.org, naota
On Thu, 2014-12-18 at 15:42 +0900, Satoru Takeuchi wrote:
> From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
>
> When "/" is Btrfs, "btrfs property <subcommand> /" regards it
> as non-root by mistake.
>
> check_is_root() regards @object as a file system root if
> the following two conditions are satisfied.
>
> a) Both @object and its parent directory are Btrfs object
> (file system root, subvolume, inode, and device
> used for Btrfs).
> b) fsid of the above mentioned two objects are different.
>
> It doesn't work if @object is "/" because, in this case,
> fsid of "/" and its parent (it's also "/"), are the same.
>
> * Test environment
>
> Two Btrfs file system (not subvolume) "/" and "/home/sat/mnt".
>
> * How to reproduce
>
> Submit "btrfs prop get" against the above mentioned file systems.
>
> * Test Result
>
> ** Actual result (without my patch)
>
> ==========================
> # btrfs prop get /home/sat/mnt/
> ro=false
> label= # label is displayed because it's a file system root
> # btrfs prop get /
> ro=false # label is not displayed even if it's a file system root
> ==========================
> ** Expected result (with my patch)
>
> ==========================
> # ./btrfs-new prop get btrfs-auto-test/
> # ./btrfs-new prop get /home/sat/mnt/
> ro=false
> label=
> # ./btrfs-new prop get /
> ro=false
> label=foo # label is displayed
> ===========================
>
> Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
> Reported-by: Naohiro Aota <naota@elisp.net>
>
> ---
> cmds-property.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/cmds-property.c b/cmds-property.c
> index a4bc127..640af99 100644
> --- a/cmds-property.c
> +++ b/cmds-property.c
> @@ -125,11 +125,22 @@ static int check_is_root(const char *object)
> u8 fsid[BTRFS_FSID_SIZE];
> u8 fsid2[BTRFS_FSID_SIZE];
> char *tmp;
> + char *rp;
> +
> + rp = realpath(object, NULL);
> + if (!rp) {
> + ret = -ENOMEM;
Hi Satoru，
IMO, ret = -errno may be better, because there are many possible return
values of realpath().
Thanks,
Gui
> + goto out;
> + }
> + if (!strcmp(rp, "/")) {
> + ret = 0;
> + goto free_rp_out;
> + }
>
> tmp = malloc(strlen(object) + 5);
> if (!tmp) {
> ret = -ENOMEM;
> - goto out;
> + goto free_rp_out;
> }
> strcpy(tmp, object);
> if (tmp[strlen(tmp) - 1] != '/')
> @@ -165,6 +176,8 @@ static int check_is_root(const char *object)
>
> free_tmp_out:
> free(tmp);
> +free_rp_out:
> + free(rp);
> out:
> return ret;
> }
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] btrfs-progs: fix the file system root is regarded as non-root
2014-12-18 7:09 ` Gui Hecheng
@ 2014-12-18 7:19 ` Satoru Takeuchi
0 siblings, 0 replies; 6+ messages in thread
From: Satoru Takeuchi @ 2014-12-18 7:19 UTC (permalink / raw)
To: Gui Hecheng; +Cc: linux-btrfs@vger.kernel.org, naota
On 2014/12/18 16:09, Gui Hecheng wrote:
> On Thu, 2014-12-18 at 15:42 +0900, Satoru Takeuchi wrote:
>> From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
>>
>> When "/" is Btrfs, "btrfs property <subcommand> /" regards it
>> as non-root by mistake.
>>
>> check_is_root() regards @object as a file system root if
>> the following two conditions are satisfied.
>>
>> a) Both @object and its parent directory are Btrfs object
>> (file system root, subvolume, inode, and device
>> used for Btrfs).
>> b) fsid of the above mentioned two objects are different.
>>
>> It doesn't work if @object is "/" because, in this case,
>> fsid of "/" and its parent (it's also "/"), are the same.
>>
>> * Test environment
>>
>> Two Btrfs file system (not subvolume) "/" and "/home/sat/mnt".
>>
>> * How to reproduce
>>
>> Submit "btrfs prop get" against the above mentioned file systems.
>>
>> * Test Result
>>
>> ** Actual result (without my patch)
>>
>> ==========================
>> # btrfs prop get /home/sat/mnt/
>> ro=false
>> label= # label is displayed because it's a file system root
>> # btrfs prop get /
>> ro=false # label is not displayed even if it's a file system root
>> ==========================
>> ** Expected result (with my patch)
>>
>> ==========================
>> # ./btrfs-new prop get btrfs-auto-test/
>> # ./btrfs-new prop get /home/sat/mnt/
>> ro=false
>> label=
>> # ./btrfs-new prop get /
>> ro=false
>> label=foo # label is displayed
>> ===========================
>>
>> Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
>> Reported-by: Naohiro Aota <naota@elisp.net>
>>
>> ---
>> cmds-property.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/cmds-property.c b/cmds-property.c
>> index a4bc127..640af99 100644
>> --- a/cmds-property.c
>> +++ b/cmds-property.c
>> @@ -125,11 +125,22 @@ static int check_is_root(const char *object)
>> u8 fsid[BTRFS_FSID_SIZE];
>> u8 fsid2[BTRFS_FSID_SIZE];
>> char *tmp;
>> + char *rp;
>> +
>> + rp = realpath(object, NULL);
>> + if (!rp) {
>> + ret = -ENOMEM;
> Hi Satoru，
>
> IMO, ret = -errno may be better, because there are many possible return
> values of realpath().
Thank you for point my mistake out.
I'll dispose 1/2 and submit 2/2 vs later.
Thanks,
Satoru
>
> Thanks,
> Gui
>
>> + goto out;
>> + }
>> + if (!strcmp(rp, "/")) {
>> + ret = 0;
>> + goto free_rp_out;
>> + }
>>
>> tmp = malloc(strlen(object) + 5);
>> if (!tmp) {
>> ret = -ENOMEM;
>> - goto out;
>> + goto free_rp_out;
>> }
>> strcpy(tmp, object);
>> if (tmp[strlen(tmp) - 1] != '/')
>> @@ -165,6 +176,8 @@ static int check_is_root(const char *object)
>>
>> free_tmp_out:
>> free(tmp);
>> +free_rp_out:
>> + free(rp);
>> out:
>> return ret;
>> }
>
>
> --
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] Fix wrong memory free on check_is_root
2014-12-18 6:09 [PATCH 1/2] Fix wrong memory free on check_is_root Satoru Takeuchi
2014-12-18 6:42 ` [PATCH 2/2] btrfs-progs: fix the file system root is regarded as non-root Satoru Takeuchi
@ 2014-12-18 7:03 ` Gui Hecheng
2014-12-18 7:16 ` Satoru Takeuchi
1 sibling, 1 reply; 6+ messages in thread
From: Gui Hecheng @ 2014-12-18 7:03 UTC (permalink / raw)
To: Satoru Takeuchi; +Cc: linux-btrfs@vger.kernel.org, naota
On Thu, 2014-12-18 at 15:09 +0900, Satoru Takeuchi wrote:
> From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
> Date: Thu, 18 Dec 2014 14:35:22 +0900
>
> @tmp is freed even if its allocation fails.
>
> Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
>
> ---
> cmds-property.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/cmds-property.c b/cmds-property.c
> index a764293..a4bc127 100644
> --- a/cmds-property.c
> +++ b/cmds-property.c
> @@ -140,31 +140,32 @@ static int check_is_root(const char *object)
Hi Satoru,
" tmp = malloc()
if (!tmp) {
...
goto out;
}
"
Actually, if the malloc() fails, it returns a NULL, and the free()
handles the NULL internally.
I think there is no special need to deal with this case by the prog
itself.
Thanks,
Gui
> if (ret < 0) {
> fprintf(stderr, "ERROR: get_fsid for %s failed. %s\n", object,
> strerror(-ret));
> - goto out;
> + goto free_tmp_out;
> }
>
> ret = get_fsid(tmp, fsid2, 1);
> if (ret == -ENOTTY) {
> ret = 0;
> - goto out;
> + goto free_tmp_out;
> } else if (ret == -ENOTDIR) {
> ret = 1;
> - goto out;
> + goto free_tmp_out;
> } else if (ret < 0) {
> fprintf(stderr, "ERROR: get_fsid for %s failed. %s\n", tmp,
> strerror(-ret));
> - goto out;
> + goto free_tmp_out;
> }
>
> if (memcmp(fsid, fsid2, BTRFS_FSID_SIZE)) {
> ret = 0;
> - goto out;
> + goto free_tmp_out;
> }
>
> ret = 1;
>
> -out:
> +free_tmp_out:
> free(tmp);
> +out:
> return ret;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] Fix wrong memory free on check_is_root
2014-12-18 7:03 ` [PATCH 1/2] Fix wrong memory free on check_is_root Gui Hecheng
@ 2014-12-18 7:16 ` Satoru Takeuchi
0 siblings, 0 replies; 6+ messages in thread
From: Satoru Takeuchi @ 2014-12-18 7:16 UTC (permalink / raw)
To: Gui Hecheng; +Cc: linux-btrfs@vger.kernel.org, naota
On 2014/12/18 16:03, Gui Hecheng wrote:
> On Thu, 2014-12-18 at 15:09 +0900, Satoru Takeuchi wrote:
>> From: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
>> Date: Thu, 18 Dec 2014 14:35:22 +0900
>>
>> @tmp is freed even if its allocation fails.
>>
>> Signed-off-by: Satoru Takeuchi <takeuchi_satoru@jp.fujitsu.com>
>>
>> ---
>> cmds-property.c | 13 +++++++------
>> 1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/cmds-property.c b/cmds-property.c
>> index a764293..a4bc127 100644
>> --- a/cmds-property.c
>> +++ b/cmds-property.c
>> @@ -140,31 +140,32 @@ static int check_is_root(const char *object)
>
> Hi Satoru,
> " tmp = malloc()
> if (!tmp) {
> ...
> goto out;
> }
> "
> Actually, if the malloc() fails, it returns a NULL, and the free()
> handles the NULL internally.
> I think there is no special need to deal with this case by the prog
> itself.
Oh, sorry... my brain would be broken today.
Thanks,
Satoru
>
> Thanks,
> Gui
>
>> if (ret < 0) {
>> fprintf(stderr, "ERROR: get_fsid for %s failed. %s\n", object,
>> strerror(-ret));
>> - goto out;
>> + goto free_tmp_out;
>> }
>>
>> ret = get_fsid(tmp, fsid2, 1);
>> if (ret == -ENOTTY) {
>> ret = 0;
>> - goto out;
>> + goto free_tmp_out;
>> } else if (ret == -ENOTDIR) {
>> ret = 1;
>> - goto out;
>> + goto free_tmp_out;
>> } else if (ret < 0) {
>> fprintf(stderr, "ERROR: get_fsid for %s failed. %s\n", tmp,
>> strerror(-ret));
>> - goto out;
>> + goto free_tmp_out;
>> }
>>
>> if (memcmp(fsid, fsid2, BTRFS_FSID_SIZE)) {
>> ret = 0;
>> - goto out;
>> + goto free_tmp_out;
>> }
>>
>> ret = 1;
>>
>> -out:
>> +free_tmp_out:
>> free(tmp);
>> +out:
>> return ret;
>> }
>>
>
>
> --
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-12-18 7:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-18 6:09 [PATCH 1/2] Fix wrong memory free on check_is_root Satoru Takeuchi
2014-12-18 6:42 ` [PATCH 2/2] btrfs-progs: fix the file system root is regarded as non-root Satoru Takeuchi
2014-12-18 7:09 ` Gui Hecheng
2014-12-18 7:19 ` Satoru Takeuchi
2014-12-18 7:03 ` [PATCH 1/2] Fix wrong memory free on check_is_root Gui Hecheng
2014-12-18 7:16 ` Satoru Takeuchi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).