* [PATCH] vfs: use kstrdup()
@ 2008-07-19 10:16 Li Zefan
2008-07-19 11:30 ` Pekka Enberg
2008-07-19 13:13 ` Cyrill Gorcunov
0 siblings, 2 replies; 20+ messages in thread
From: Li Zefan @ 2008-07-19 10:16 UTC (permalink / raw)
To: Andrew Morton; +Cc: Al Viro, LKML
This piece of code can be replaced by a kstrdup().
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
fs/namespace.c | 9 +--------
1 files changed, 1 insertions(+), 8 deletions(-)
diff --git a/fs/namespace.c b/fs/namespace.c
index 4f6f763..08c3b2d 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -127,14 +127,7 @@ struct vfsmount *alloc_vfsmnt(const char *name)
INIT_LIST_HEAD(&mnt->mnt_slave_list);
INIT_LIST_HEAD(&mnt->mnt_slave);
atomic_set(&mnt->__mnt_writers, 0);
- if (name) {
- int size = strlen(name) + 1;
- char *newname = kmalloc(size, GFP_KERNEL);
- if (newname) {
- memcpy(newname, name, size);
- mnt->mnt_devname = newname;
- }
- }
+ mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
}
return mnt;
}
--
1.5.4.rc3
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-19 10:16 [PATCH] vfs: use kstrdup() Li Zefan
@ 2008-07-19 11:30 ` Pekka Enberg
2008-07-19 13:13 ` Cyrill Gorcunov
1 sibling, 0 replies; 20+ messages in thread
From: Pekka Enberg @ 2008-07-19 11:30 UTC (permalink / raw)
To: Li Zefan; +Cc: Andrew Morton, Al Viro, LKML
On Sat, Jul 19, 2008 at 1:16 PM, Li Zefan <lizf@cn.fujitsu.com> wrote:
> This piece of code can be replaced by a kstrdup().
>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Looks good to me.
Reviewed-by: Pekka Enberg <penberg@cs.helsinki.fi>
> ---
> fs/namespace.c | 9 +--------
> 1 files changed, 1 insertions(+), 8 deletions(-)
>
> diff --git a/fs/namespace.c b/fs/namespace.c
> index 4f6f763..08c3b2d 100644
> --- a/fs/namespace.c
> +++ b/fs/namespace.c
> @@ -127,14 +127,7 @@ struct vfsmount *alloc_vfsmnt(const char *name)
> INIT_LIST_HEAD(&mnt->mnt_slave_list);
> INIT_LIST_HEAD(&mnt->mnt_slave);
> atomic_set(&mnt->__mnt_writers, 0);
> - if (name) {
> - int size = strlen(name) + 1;
> - char *newname = kmalloc(size, GFP_KERNEL);
> - if (newname) {
> - memcpy(newname, name, size);
> - mnt->mnt_devname = newname;
> - }
> - }
> + mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
> }
> return mnt;
> }
> --
> 1.5.4.rc3
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-19 10:16 [PATCH] vfs: use kstrdup() Li Zefan
2008-07-19 11:30 ` Pekka Enberg
@ 2008-07-19 13:13 ` Cyrill Gorcunov
2008-07-19 13:19 ` Cyrill Gorcunov
1 sibling, 1 reply; 20+ messages in thread
From: Cyrill Gorcunov @ 2008-07-19 13:13 UTC (permalink / raw)
To: Li Zefan; +Cc: Andrew Morton, Al Viro, LKML
[Li Zefan - Sat, Jul 19, 2008 at 06:16:20PM +0800]
| This piece of code can be replaced by a kstrdup().
|
| Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
| ---
| fs/namespace.c | 9 +--------
| 1 files changed, 1 insertions(+), 8 deletions(-)
|
| diff --git a/fs/namespace.c b/fs/namespace.c
| index 4f6f763..08c3b2d 100644
| --- a/fs/namespace.c
| +++ b/fs/namespace.c
| @@ -127,14 +127,7 @@ struct vfsmount *alloc_vfsmnt(const char *name)
| INIT_LIST_HEAD(&mnt->mnt_slave_list);
| INIT_LIST_HEAD(&mnt->mnt_slave);
| atomic_set(&mnt->__mnt_writers, 0);
| - if (name) {
| - int size = strlen(name) + 1;
| - char *newname = kmalloc(size, GFP_KERNEL);
| - if (newname) {
| - memcpy(newname, name, size);
| - mnt->mnt_devname = newname;
| - }
| - }
| + mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
| }
| return mnt;
| }
| --
| 1.5.4.rc3
|
|
but kstrdup may return NULL - is it safe there?
Sorry if that "not smart" question.
- Cyrill -
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-19 13:13 ` Cyrill Gorcunov
@ 2008-07-19 13:19 ` Cyrill Gorcunov
2008-07-21 5:27 ` Al Viro
0 siblings, 1 reply; 20+ messages in thread
From: Cyrill Gorcunov @ 2008-07-19 13:19 UTC (permalink / raw)
To: Li Zefan, Andrew Morton, Al Viro, LKML
[Cyrill Gorcunov - Sat, Jul 19, 2008 at 05:13:17PM +0400]
[...]
| | - }
| | - }
| | + mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
| | }
| | return mnt;
| | }
| | --
| | 1.5.4.rc3
| |
| |
|
| but kstrdup may return NULL - is it safe there?
| Sorry if that "not smart" question.
|
| - Cyrill -
ah, I see it is safe - sorry for noise
- Cyrill -
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-19 13:19 ` Cyrill Gorcunov
@ 2008-07-21 5:27 ` Al Viro
2008-07-21 6:29 ` Li Zefan
0 siblings, 1 reply; 20+ messages in thread
From: Al Viro @ 2008-07-21 5:27 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: Li Zefan, Andrew Morton, LKML
On Sat, Jul 19, 2008 at 05:19:09PM +0400, Cyrill Gorcunov wrote:
> [Cyrill Gorcunov - Sat, Jul 19, 2008 at 05:13:17PM +0400]
> [...]
> | | - }
> | | - }
> | | + mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
> | | }
> | | return mnt;
> | | }
> | | --
> | | 1.5.4.rc3
> | |
> | |
> |
> | but kstrdup may return NULL - is it safe there?
> | Sorry if that "not smart" question.
> |
> | - Cyrill -
>
> ah, I see it is safe - sorry for noise
FWIW, it _is_ a good question.
* is all code treating ->mnt_devname as optional? AFAICS, there's
at least one place in NFS that doesn't. We could treat failing allocation
the same way we treat failing allocation of vfsmount itself - callers can
cope with that already.
* AFAICS, it should be const char *.
* ... or perhaps we shouldn't copy it at all. How about something
like
struct {
int count;
char name[];
}
with cloning sharing the reference and bumping the count, protecting it with
e.g. vfsmount_lock? For setups where we have a lot of bindings/namespaces
it might be noticable.
Any takers?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 5:27 ` Al Viro
@ 2008-07-21 6:29 ` Li Zefan
2008-07-21 7:03 ` Al Viro
0 siblings, 1 reply; 20+ messages in thread
From: Li Zefan @ 2008-07-21 6:29 UTC (permalink / raw)
To: Al Viro; +Cc: Cyrill Gorcunov, Andrew Morton, LKML
Al Viro wrote:
> On Sat, Jul 19, 2008 at 05:19:09PM +0400, Cyrill Gorcunov wrote:
>> [Cyrill Gorcunov - Sat, Jul 19, 2008 at 05:13:17PM +0400]
>> [...]
>> | | - }
>> | | - }
>> | | + mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
>> | | }
>> | | return mnt;
>> | | }
>> | | --
>> | | 1.5.4.rc3
>> | |
>> | |
>> |
>> | but kstrdup may return NULL - is it safe there?
>> | Sorry if that "not smart" question.
>> |
>> | - Cyrill -
>>
>> ah, I see it is safe - sorry for noise
>
> FWIW, it _is_ a good question.
>
> * is all code treating ->mnt_devname as optional? AFAICS, there's
> at least one place in NFS that doesn't. We could treat failing allocation
> the same way we treat failing allocation of vfsmount itself - callers can
> cope with that already.
I just did a cleanup, and the original code didn't check for NULL.
I just looked into the git history, and I found out since fs/namespace.c was
created in v2.4.10.4, the code has never changed to check for failing
allocation of ->mnt_devname.
> * AFAICS, it should be const char *.
Agreed.
> * ... or perhaps we shouldn't copy it at all. How about something
> like
> struct {
> int count;
> char name[];
> }
> with cloning sharing the reference and bumping the count, protecting it with
> e.g. vfsmount_lock? For setups where we have a lot of bindings/namespaces
> it might be noticable.
>
I'm not sure whether this is a good idea, as I have limited knowledge about the
vfs internal.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 6:29 ` Li Zefan
@ 2008-07-21 7:03 ` Al Viro
2008-07-21 7:09 ` Cyrill Gorcunov
2008-07-21 8:04 ` Cyrill Gorcunov
0 siblings, 2 replies; 20+ messages in thread
From: Al Viro @ 2008-07-21 7:03 UTC (permalink / raw)
To: Li Zefan; +Cc: Cyrill Gorcunov, Andrew Morton, LKML
On Mon, Jul 21, 2008 at 02:29:47PM +0800, Li Zefan wrote:
> > FWIW, it _is_ a good question.
> >
> > * is all code treating ->mnt_devname as optional? AFAICS, there's
> > at least one place in NFS that doesn't. We could treat failing allocation
> > the same way we treat failing allocation of vfsmount itself - callers can
> > cope with that already.
>
> I just did a cleanup, and the original code didn't check for NULL.
I know.
> I just looked into the git history, and I found out since fs/namespace.c was
> created in v2.4.10.4, the code has never changed to check for failing
> allocation of ->mnt_devname.
It used to have no users beyond fs/namespace.c itself and for _those_ the
thing had been optional, so leaving NULL had been OK. Unfortunately, it
still had been a bad idea - new users had appeared and those predictably
didn't notice that fun detail.
The right thing here is to consider failing allocation of ->mnt_devname
as failure of the entire alloc.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 7:03 ` Al Viro
@ 2008-07-21 7:09 ` Cyrill Gorcunov
2008-07-21 8:04 ` Cyrill Gorcunov
1 sibling, 0 replies; 20+ messages in thread
From: Cyrill Gorcunov @ 2008-07-21 7:09 UTC (permalink / raw)
To: Al Viro; +Cc: Li Zefan, Andrew Morton, LKML
[Al Viro - Mon, Jul 21, 2008 at 08:03:46AM +0100]
| On Mon, Jul 21, 2008 at 02:29:47PM +0800, Li Zefan wrote:
| > > FWIW, it _is_ a good question.
| > >
| > > * is all code treating ->mnt_devname as optional? AFAICS, there's
| > > at least one place in NFS that doesn't. We could treat failing allocation
| > > the same way we treat failing allocation of vfsmount itself - callers can
| > > cope with that already.
| >
| > I just did a cleanup, and the original code didn't check for NULL.
|
| I know.
|
| > I just looked into the git history, and I found out since fs/namespace.c was
| > created in v2.4.10.4, the code has never changed to check for failing
| > allocation of ->mnt_devname.
|
| It used to have no users beyond fs/namespace.c itself and for _those_ the
| thing had been optional, so leaving NULL had been OK. Unfortunately, it
| still had been a bad idea - new users had appeared and those predictably
| didn't notice that fun detail.
|
| The right thing here is to consider failing allocation of ->mnt_devname
| as failure of the entire alloc.
|
Hi Al,
thanks a lot for comments! I think it is more then enough for now (i'm
about failing allocation in whole). If that happens and we are not able
to duplicate string - it's quite probable we will be in serious troubles
soon anyway ('cause of further kmalloc calls). So it's better to get
mount allocation fails then NULL deref.
- Cyrill -
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 7:03 ` Al Viro
2008-07-21 7:09 ` Cyrill Gorcunov
@ 2008-07-21 8:04 ` Cyrill Gorcunov
2008-07-21 8:28 ` Li Zefan
2008-07-21 8:28 ` Al Viro
1 sibling, 2 replies; 20+ messages in thread
From: Cyrill Gorcunov @ 2008-07-21 8:04 UTC (permalink / raw)
To: Al Viro; +Cc: Li Zefan, Andrew Morton, LKML
[Al Viro - Mon, Jul 21, 2008 at 08:03:46AM +0100]
|
[...]
| The right thing here is to consider failing allocation of ->mnt_devname
| as failure of the entire alloc.
|
Al, what about the patch below? I'm not sure if Li's version already
in someone tree so it's from-the-scratch. If this ok, i think Li could
update his version and resend.
- Cyrill -
---
Index: linux-2.6.git/fs/namespace.c
===================================================================
--- linux-2.6.git.orig/fs/namespace.c 2008-07-21 11:34:37.000000000 +0400
+++ linux-2.6.git/fs/namespace.c 2008-07-21 12:00:01.000000000 +0400
@@ -112,9 +112,13 @@ struct vfsmount *alloc_vfsmnt(const char
int err;
err = mnt_alloc_id(mnt);
- if (err) {
- kmem_cache_free(mnt_cache, mnt);
- return NULL;
+ if (err)
+ goto err;
+
+ if (name) {
+ mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
+ if (!mnt->mnt_devname)
+ goto err;
}
atomic_set(&mnt->mnt_count, 1);
@@ -127,16 +131,12 @@ struct vfsmount *alloc_vfsmnt(const char
INIT_LIST_HEAD(&mnt->mnt_slave_list);
INIT_LIST_HEAD(&mnt->mnt_slave);
atomic_set(&mnt->__mnt_writers, 0);
- if (name) {
- int size = strlen(name) + 1;
- char *newname = kmalloc(size, GFP_KERNEL);
- if (newname) {
- memcpy(newname, name, size);
- mnt->mnt_devname = newname;
- }
- }
}
return mnt;
+
+err:
+ kmem_cache_free(mnt_cache, mnt);
+ return NULL;
}
/*
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 8:04 ` Cyrill Gorcunov
@ 2008-07-21 8:28 ` Li Zefan
2008-07-21 8:44 ` Cyrill Gorcunov
2008-07-21 8:28 ` Al Viro
1 sibling, 1 reply; 20+ messages in thread
From: Li Zefan @ 2008-07-21 8:28 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: Al Viro, Andrew Morton, LKML
Cyrill Gorcunov wrote:
> [Al Viro - Mon, Jul 21, 2008 at 08:03:46AM +0100]
> |
> [...]
> | The right thing here is to consider failing allocation of ->mnt_devname
> | as failure of the entire alloc.
> |
>
> Al, what about the patch below? I'm not sure if Li's version already
> in someone tree so it's from-the-scratch. If this ok, i think Li could
> update his version and resend.
>
It's already in -mm tree, but Andrew can drop it and queue the new one.
> - Cyrill -
> ---
>
> Index: linux-2.6.git/fs/namespace.c
> ===================================================================
> --- linux-2.6.git.orig/fs/namespace.c 2008-07-21 11:34:37.000000000 +0400
> +++ linux-2.6.git/fs/namespace.c 2008-07-21 12:00:01.000000000 +0400
> @@ -112,9 +112,13 @@ struct vfsmount *alloc_vfsmnt(const char
> int err;
>
> err = mnt_alloc_id(mnt);
> - if (err) {
> - kmem_cache_free(mnt_cache, mnt);
> - return NULL;
> + if (err)
> + goto err;
> +
> + if (name) {
> + mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
> + if (!mnt->mnt_devname)
should call mnt_free_id() here.
> + goto err;
> }
>
> atomic_set(&mnt->mnt_count, 1);
> @@ -127,16 +131,12 @@ struct vfsmount *alloc_vfsmnt(const char
> INIT_LIST_HEAD(&mnt->mnt_slave_list);
> INIT_LIST_HEAD(&mnt->mnt_slave);
> atomic_set(&mnt->__mnt_writers, 0);
> - if (name) {
> - int size = strlen(name) + 1;
> - char *newname = kmalloc(size, GFP_KERNEL);
> - if (newname) {
> - memcpy(newname, name, size);
> - mnt->mnt_devname = newname;
> - }
> - }
> }
> return mnt;
> +
> +err:
> + kmem_cache_free(mnt_cache, mnt);
> + return NULL;
> }
>
> /*
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 8:04 ` Cyrill Gorcunov
2008-07-21 8:28 ` Li Zefan
@ 2008-07-21 8:28 ` Al Viro
2008-07-21 8:35 ` Cyrill Gorcunov
2008-07-21 8:42 ` Cyrill Gorcunov
1 sibling, 2 replies; 20+ messages in thread
From: Al Viro @ 2008-07-21 8:28 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: Li Zefan, Andrew Morton, LKML
On Mon, Jul 21, 2008 at 12:04:27PM +0400, Cyrill Gorcunov wrote:
> err = mnt_alloc_id(mnt);
> - if (err) {
> - kmem_cache_free(mnt_cache, mnt);
> - return NULL;
> + if (err)
> + goto err;
Ugh... Labels are in a separate namespace, but really...
> + if (name) {
> + mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
> + if (!mnt->mnt_devname)
> + goto err;
> +err:
> + kmem_cache_free(mnt_cache, mnt);
> + return NULL;
Leak; note the mnt_alloc_id() above. Either do that kstrdup() first and
kfree the result on mnt_alloc_id() failure or do mnt_free_id() on kstrdup()
one...
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 8:28 ` Al Viro
@ 2008-07-21 8:35 ` Cyrill Gorcunov
2008-07-21 8:42 ` Cyrill Gorcunov
1 sibling, 0 replies; 20+ messages in thread
From: Cyrill Gorcunov @ 2008-07-21 8:35 UTC (permalink / raw)
To: Al Viro; +Cc: Li Zefan, Andrew Morton, LKML
[Al Viro - Mon, Jul 21, 2008 at 09:28:57AM +0100]
| On Mon, Jul 21, 2008 at 12:04:27PM +0400, Cyrill Gorcunov wrote:
| > err = mnt_alloc_id(mnt);
| > - if (err) {
| > - kmem_cache_free(mnt_cache, mnt);
| > - return NULL;
| > + if (err)
| > + goto err;
|
| Ugh... Labels are in a separate namespace, but really...
|
| > + if (name) {
| > + mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
| > + if (!mnt->mnt_devname)
| > + goto err;
|
| > +err:
| > + kmem_cache_free(mnt_cache, mnt);
| > + return NULL;
|
| Leak; note the mnt_alloc_id() above. Either do that kstrdup() first and
| kfree the result on mnt_alloc_id() failure or do mnt_free_id() on kstrdup()
| one...
|
oh my, that stupid from my side - will update shortly
- Cyrill -
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 8:28 ` Al Viro
2008-07-21 8:35 ` Cyrill Gorcunov
@ 2008-07-21 8:42 ` Cyrill Gorcunov
1 sibling, 0 replies; 20+ messages in thread
From: Cyrill Gorcunov @ 2008-07-21 8:42 UTC (permalink / raw)
To: Al Viro; +Cc: Li Zefan, Andrew Morton, LKML
[Al Viro - Mon, Jul 21, 2008 at 09:28:57AM +0100]
| On Mon, Jul 21, 2008 at 12:04:27PM +0400, Cyrill Gorcunov wrote:
| > err = mnt_alloc_id(mnt);
| > - if (err) {
| > - kmem_cache_free(mnt_cache, mnt);
| > - return NULL;
| > + if (err)
| > + goto err;
|
| Ugh... Labels are in a separate namespace, but really...
|
| > + if (name) {
| > + mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
| > + if (!mnt->mnt_devname)
| > + goto err;
|
| > +err:
| > + kmem_cache_free(mnt_cache, mnt);
| > + return NULL;
|
| Leak; note the mnt_alloc_id() above. Either do that kstrdup() first and
| kfree the result on mnt_alloc_id() failure or do mnt_free_id() on kstrdup()
| one...
|
ok, here is an update version. labels names could be not that good
choosen (check please).
- Cyrill -
---
Index: linux-2.6.git/fs/namespace.c
===================================================================
--- linux-2.6.git.orig/fs/namespace.c 2008-07-21 11:34:37.000000000 +0400
+++ linux-2.6.git/fs/namespace.c 2008-07-21 12:40:12.000000000 +0400
@@ -112,9 +112,13 @@ struct vfsmount *alloc_vfsmnt(const char
int err;
err = mnt_alloc_id(mnt);
- if (err) {
- kmem_cache_free(mnt_cache, mnt);
- return NULL;
+ if (err)
+ goto out_free_cache;
+
+ if (name) {
+ mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
+ if (!mnt->mnt_devname)
+ goto out_free_id;
}
atomic_set(&mnt->mnt_count, 1);
@@ -127,16 +131,14 @@ struct vfsmount *alloc_vfsmnt(const char
INIT_LIST_HEAD(&mnt->mnt_slave_list);
INIT_LIST_HEAD(&mnt->mnt_slave);
atomic_set(&mnt->__mnt_writers, 0);
- if (name) {
- int size = strlen(name) + 1;
- char *newname = kmalloc(size, GFP_KERNEL);
- if (newname) {
- memcpy(newname, name, size);
- mnt->mnt_devname = newname;
- }
- }
}
return mnt;
+
+out_free_id:
+ mnt_free_id(mnt);
+out_free_cache:
+ kmem_cache_free(mnt_cache, mnt);
+ return NULL;
}
/*
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 8:28 ` Li Zefan
@ 2008-07-21 8:44 ` Cyrill Gorcunov
2008-07-21 8:59 ` Li Zefan
0 siblings, 1 reply; 20+ messages in thread
From: Cyrill Gorcunov @ 2008-07-21 8:44 UTC (permalink / raw)
To: Li Zefan; +Cc: Al Viro, Andrew Morton, LKML
[Li Zefan - Mon, Jul 21, 2008 at 04:28:17PM +0800]
| Cyrill Gorcunov wrote:
| > [Al Viro - Mon, Jul 21, 2008 at 08:03:46AM +0100]
| > |
| > [...]
| > | The right thing here is to consider failing allocation of ->mnt_devname
| > | as failure of the entire alloc.
| > |
| >
| > Al, what about the patch below? I'm not sure if Li's version already
| > in someone tree so it's from-the-scratch. If this ok, i think Li could
| > update his version and resend.
| >
|
| It's already in -mm tree, but Andrew can drop it and queue the new one.
|
| > - Cyrill -
| > ---
| >
| > Index: linux-2.6.git/fs/namespace.c
| > ===================================================================
| > --- linux-2.6.git.orig/fs/namespace.c 2008-07-21 11:34:37.000000000 +0400
| > +++ linux-2.6.git/fs/namespace.c 2008-07-21 12:00:01.000000000 +0400
| > @@ -112,9 +112,13 @@ struct vfsmount *alloc_vfsmnt(const char
| > int err;
| >
| > err = mnt_alloc_id(mnt);
| > - if (err) {
| > - kmem_cache_free(mnt_cache, mnt);
| > - return NULL;
| > + if (err)
| > + goto err;
| > +
| > + if (name) {
| > + mnt->mnt_devname = kstrdup(name, GFP_KERNEL);
| > + if (!mnt->mnt_devname)
|
| should call mnt_free_id() here.
|
| > + goto err;
| > }
| >
| > atomic_set(&mnt->mnt_count, 1);
| > @@ -127,16 +131,12 @@ struct vfsmount *alloc_vfsmnt(const char
| > INIT_LIST_HEAD(&mnt->mnt_slave_list);
| > INIT_LIST_HEAD(&mnt->mnt_slave);
| > atomic_set(&mnt->__mnt_writers, 0);
| > - if (name) {
| > - int size = strlen(name) + 1;
| > - char *newname = kmalloc(size, GFP_KERNEL);
| > - if (newname) {
| > - memcpy(newname, name, size);
| > - mnt->mnt_devname = newname;
| > - }
| > - }
| > }
| > return mnt;
| > +
| > +err:
| > + kmem_cache_free(mnt_cache, mnt);
| > + return NULL;
| > }
| >
| > /*
| >
| >
|
well, as only Al find new version more or less correct we could
update yours.
- Cyrill -
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 8:44 ` Cyrill Gorcunov
@ 2008-07-21 8:59 ` Li Zefan
2008-07-21 9:06 ` Pekka Enberg
2008-07-21 9:12 ` Al Viro
0 siblings, 2 replies; 20+ messages in thread
From: Li Zefan @ 2008-07-21 8:59 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: Al Viro, Andrew Morton, LKML
> well, as only Al find new version more or less correct we could
> update yours.
>
Sorry, I don't quite catch what you mean for my poor English. :(
Do you expect me to update and resend my version? or do you mean don't
drop the patch in -mm but you will resend a patch based against it?
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 8:59 ` Li Zefan
@ 2008-07-21 9:06 ` Pekka Enberg
2008-07-21 10:05 ` Cyrill Gorcunov
2008-07-21 9:12 ` Al Viro
1 sibling, 1 reply; 20+ messages in thread
From: Pekka Enberg @ 2008-07-21 9:06 UTC (permalink / raw)
To: Li Zefan; +Cc: Cyrill Gorcunov, Al Viro, Andrew Morton, LKML
Hi,
On Mon, Jul 21, 2008 at 11:59 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
>> well, as only Al find new version more or less correct we could
>> update yours.
>
> Sorry, I don't quite catch what you mean for my poor English. :(
>
> Do you expect me to update and resend my version? or do you mean don't
> drop the patch in -mm but you will resend a patch based against it?
Andrew already merged your patch so I think Cyrill should just send a
new patch against -mm.
Pekka
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 8:59 ` Li Zefan
2008-07-21 9:06 ` Pekka Enberg
@ 2008-07-21 9:12 ` Al Viro
1 sibling, 0 replies; 20+ messages in thread
From: Al Viro @ 2008-07-21 9:12 UTC (permalink / raw)
To: Li Zefan; +Cc: Cyrill Gorcunov, Andrew Morton, LKML
On Mon, Jul 21, 2008 at 04:59:16PM +0800, Li Zefan wrote:
> > well, as only Al find new version more or less correct we could
> > update yours.
> >
>
> Sorry, I don't quite catch what you mean for my poor English. :(
>
> Do you expect me to update and resend my version? or do you mean don't
> drop the patch in -mm but you will resend a patch based against it?
<shrug>
FWIW, the latest posted variant looks sane. Whether you want it in one
piece or two... up to you, folks. Whichever way you do it, throw the
char * -> const char * transition for mnt_devname into the mix and send
the prefered splitup; I'll apply it after I get some sleep...
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 9:06 ` Pekka Enberg
@ 2008-07-21 10:05 ` Cyrill Gorcunov
2008-07-21 10:10 ` Li Zefan
0 siblings, 1 reply; 20+ messages in thread
From: Cyrill Gorcunov @ 2008-07-21 10:05 UTC (permalink / raw)
To: Pekka Enberg; +Cc: Li Zefan, Al Viro, Andrew Morton, LKML
[Pekka Enberg - Mon, Jul 21, 2008 at 12:06:55PM +0300]
| Hi,
|
| On Mon, Jul 21, 2008 at 11:59 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
| >> well, as only Al find new version more or less correct we could
| >> update yours.
| >
| > Sorry, I don't quite catch what you mean for my poor English. :(
| >
| > Do you expect me to update and resend my version? or do you mean don't
| > drop the patch in -mm but you will resend a patch based against it?
|
| Andrew already merged your patch so I think Cyrill should just send a
| new patch against -mm.
|
| Pekka
|
Thanks guys, will send patch against -mm soon (a bit busy now)
- Cyrill -
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 10:05 ` Cyrill Gorcunov
@ 2008-07-21 10:10 ` Li Zefan
2008-07-21 10:17 ` Cyrill Gorcunov
0 siblings, 1 reply; 20+ messages in thread
From: Li Zefan @ 2008-07-21 10:10 UTC (permalink / raw)
To: Cyrill Gorcunov; +Cc: Pekka Enberg, Al Viro, Andrew Morton, LKML
Cyrill Gorcunov wrote:
> [Pekka Enberg - Mon, Jul 21, 2008 at 12:06:55PM +0300]
> | Hi,
> |
> | On Mon, Jul 21, 2008 at 11:59 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
> | >> well, as only Al find new version more or less correct we could
> | >> update yours.
> | >
> | > Sorry, I don't quite catch what you mean for my poor English. :(
> | >
> | > Do you expect me to update and resend my version? or do you mean don't
> | > drop the patch in -mm but you will resend a patch based against it?
> |
> | Andrew already merged your patch so I think Cyrill should just send a
> | new patch against -mm.
> |
> | Pekka
> |
>
> Thanks guys, will send patch against -mm soon (a bit busy now)
>
Oh, I just sent out a mix-up patch because I'm going to get off work but I didn't
get your respond, you can just ignore it.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] vfs: use kstrdup()
2008-07-21 10:10 ` Li Zefan
@ 2008-07-21 10:17 ` Cyrill Gorcunov
0 siblings, 0 replies; 20+ messages in thread
From: Cyrill Gorcunov @ 2008-07-21 10:17 UTC (permalink / raw)
To: Li Zefan; +Cc: Pekka Enberg, Al Viro, Andrew Morton, LKML
[Li Zefan - Mon, Jul 21, 2008 at 06:10:19PM +0800]
| Cyrill Gorcunov wrote:
| > [Pekka Enberg - Mon, Jul 21, 2008 at 12:06:55PM +0300]
| > | Hi,
| > |
| > | On Mon, Jul 21, 2008 at 11:59 AM, Li Zefan <lizf@cn.fujitsu.com> wrote:
| > | >> well, as only Al find new version more or less correct we could
| > | >> update yours.
| > | >
| > | > Sorry, I don't quite catch what you mean for my poor English. :(
| > | >
| > | > Do you expect me to update and resend my version? or do you mean don't
| > | > drop the patch in -mm but you will resend a patch based against it?
| > |
| > | Andrew already merged your patch so I think Cyrill should just send a
| > | new patch against -mm.
| > |
| > | Pekka
| > |
| >
| > Thanks guys, will send patch against -mm soon (a bit busy now)
| >
|
| Oh, I just sent out a mix-up patch because I'm going to get off work but I didn't
| get your respond, you can just ignore it.
|
it's ok - since you did my work for me :) Thanks.
- Cyrill -
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2008-07-21 10:17 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-07-19 10:16 [PATCH] vfs: use kstrdup() Li Zefan
2008-07-19 11:30 ` Pekka Enberg
2008-07-19 13:13 ` Cyrill Gorcunov
2008-07-19 13:19 ` Cyrill Gorcunov
2008-07-21 5:27 ` Al Viro
2008-07-21 6:29 ` Li Zefan
2008-07-21 7:03 ` Al Viro
2008-07-21 7:09 ` Cyrill Gorcunov
2008-07-21 8:04 ` Cyrill Gorcunov
2008-07-21 8:28 ` Li Zefan
2008-07-21 8:44 ` Cyrill Gorcunov
2008-07-21 8:59 ` Li Zefan
2008-07-21 9:06 ` Pekka Enberg
2008-07-21 10:05 ` Cyrill Gorcunov
2008-07-21 10:10 ` Li Zefan
2008-07-21 10:17 ` Cyrill Gorcunov
2008-07-21 9:12 ` Al Viro
2008-07-21 8:28 ` Al Viro
2008-07-21 8:35 ` Cyrill Gorcunov
2008-07-21 8:42 ` Cyrill Gorcunov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox