Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
On Wed, Apr 11, 2007 at 02:23:49PM +0800, Fengguang Wu wrote: >On Sun, Apr 01, 2007 at 02:20:46PM +0800, Cong WANG wrote: >> 2007/4/1, Andrew Morton <[EMAIL PROTECTED]>: >> > >> >Also, please always prepare patches in `patch -p1' form, as per >> >http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks. >> > >> > >> >> Sorry. I am confused with this. Does that mean I should make patches >> _upon_ the root kernel source directory or first make a copy of the >> original source code and then diff against the two dirs? But I was >> told that "patches should be based _in_ the root kernel source >> directory" and when only one file was modified just to diff it with >> the original single file. (See Documentation/SubmittingPatches.) >> >> Can you help out? And should I remake this patch? Thanks again! > >quilt(http://savannah.nongnu.org/projects/quilt) is your friend. > >cd /usr/src/linux >quilt new my-fix.patch >quilt edit mm/readahead.c >quilt refresh --diffstat >quilt diff >... Thanks. I will try it. ;-) - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
On Sun, Apr 01, 2007 at 02:20:46PM +0800, Cong WANG wrote: > 2007/4/1, Andrew Morton <[EMAIL PROTECTED]>: > > > >Also, please always prepare patches in `patch -p1' form, as per > >http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks. > > > > > > Sorry. I am confused with this. Does that mean I should make patches > _upon_ the root kernel source directory or first make a copy of the > original source code and then diff against the two dirs? But I was > told that "patches should be based _in_ the root kernel source > directory" and when only one file was modified just to diff it with > the original single file. (See Documentation/SubmittingPatches.) > > Can you help out? And should I remake this patch? Thanks again! quilt(http://savannah.nongnu.org/projects/quilt) is your friend. cd /usr/src/linux quilt new my-fix.patch quilt edit mm/readahead.c quilt refresh --diffstat quilt diff ... - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
On Sun, Apr 01, 2007 at 02:20:46PM +0800, Cong WANG wrote: 2007/4/1, Andrew Morton [EMAIL PROTECTED]: Also, please always prepare patches in `patch -p1' form, as per http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks. Sorry. I am confused with this. Does that mean I should make patches _upon_ the root kernel source directory or first make a copy of the original source code and then diff against the two dirs? But I was told that patches should be based _in_ the root kernel source directory and when only one file was modified just to diff it with the original single file. (See Documentation/SubmittingPatches.) Can you help out? And should I remake this patch? Thanks again! quilt(http://savannah.nongnu.org/projects/quilt) is your friend. cd /usr/src/linux quilt new my-fix.patch quilt edit mm/readahead.c quilt refresh --diffstat quilt diff ... - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
On Wed, Apr 11, 2007 at 02:23:49PM +0800, Fengguang Wu wrote: On Sun, Apr 01, 2007 at 02:20:46PM +0800, Cong WANG wrote: 2007/4/1, Andrew Morton [EMAIL PROTECTED]: Also, please always prepare patches in `patch -p1' form, as per http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks. Sorry. I am confused with this. Does that mean I should make patches _upon_ the root kernel source directory or first make a copy of the original source code and then diff against the two dirs? But I was told that patches should be based _in_ the root kernel source directory and when only one file was modified just to diff it with the original single file. (See Documentation/SubmittingPatches.) Can you help out? And should I remake this patch? Thanks again! quilt(http://savannah.nongnu.org/projects/quilt) is your friend. cd /usr/src/linux quilt new my-fix.patch quilt edit mm/readahead.c quilt refresh --diffstat quilt diff ... Thanks. I will try it. ;-) - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
2007/4/1, Andrew Morton <[EMAIL PROTECTED]>: On Sun, 1 Apr 2007 14:20:46 +0800 "Cong WANG" <[EMAIL PROTECTED]> wrote: > > > > > Also, please always prepare patches in `patch -p1' form, as per > > http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks. > > > > > > Sorry. I am confused with this. Does that mean I should make patches > _upon_ the root kernel source directory or first make a copy of the > original source code and then diff against the two dirs? But I was > told that "patches should be based _in_ the root kernel source > directory" and when only one file was modified just to diff it with > the original single file. (See Documentation/SubmittingPatches.) The headers should look like: --- a/arch/cris/kernel/crisksyms.c +++ a/arch/cris/kernel/crisksyms.c I don't know how people do that. One obvious way is to do cd /usr/src diff -u linux-orig/arch/cris/kernel/crisksyms.c linux-new/arch/cris/kernel/crisksyms.c other people probably alter the diff headers. Oh, thanks. I know. > And should I remake this patch? Sure, but please change it to perform correct error handling first. And test that error handling, if you can. That will involve adding artificial errors. I will remake it soon and send it again. In fact, I have just tested it roughly. And I don't how to produce artificial errors. Sorry, I hope someone can help me to test it carefully. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
On Sun, 1 Apr 2007 14:20:46 +0800 "Cong WANG" <[EMAIL PROTECTED]> wrote: > > > > > Also, please always prepare patches in `patch -p1' form, as per > > http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks. > > > > > > Sorry. I am confused with this. Does that mean I should make patches > _upon_ the root kernel source directory or first make a copy of the > original source code and then diff against the two dirs? But I was > told that "patches should be based _in_ the root kernel source > directory" and when only one file was modified just to diff it with > the original single file. (See Documentation/SubmittingPatches.) The headers should look like: --- a/arch/cris/kernel/crisksyms.c +++ a/arch/cris/kernel/crisksyms.c I don't know how people do that. One obvious way is to do cd /usr/src diff -u linux-orig/arch/cris/kernel/crisksyms.c linux-new/arch/cris/kernel/crisksyms.c other people probably alter the diff headers. > And should I remake this patch? Sure, but please change it to perform correct error handling first. And test that error handling, if you can. That will involve adding artificial errors. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
2007/4/1, Andrew Morton <[EMAIL PROTECTED]>:
On Sat, 31 Mar 2007 10:30:31 +0800 "Cong WANG" <[EMAIL PROTECTED]> wrote:
> Since kobject_add, sysfs_create_link and sysfs_create_file are marked
> as '__must_check', so we must always check their return values, or gcc
> will give us warnings.
>
> Signed-off-by: Cong WANG <[EMAIL PROTECTED]>
>
> ---
> --- fs/partitions/check.c.orig2007-03-30 21:35:45.0 +0800
> +++ fs/partitions/check.c 2007-03-30 21:49:53.0 +0800
> @@ -385,10 +385,16 @@ void add_partition(struct gendisk *disk,
> p->kobj.parent = >kobj;
> p->kobj.ktype = _part;
> kobject_init(>kobj);
> - kobject_add(>kobj);
> + if (kobject_add(>kobj)) {
> + kfree(p);
> + return;
> + }
> if (!disk->part_uevent_suppress)
> kobject_uevent(>kobj, KOBJ_ADD);
> - sysfs_create_link(>kobj, _subsys.kset.kobj, "subsystem");
> + if (sysfs_create_link(>kobj, _subsys.kset.kobj, "subsystem")) {
> + kfree(p);
> + return;
> + }
> if (flags & ADDPART_FLAG_WHOLEDISK) {
> static struct attribute addpartattr = {
> .name = "whole_disk",
> @@ -396,7 +402,10 @@ void add_partition(struct gendisk *disk,
> .owner = THIS_MODULE,
> };
>
> - sysfs_create_file(>kobj, );
> + if (sysfs_create_file(>kobj, )) {
> + kfree(p);
> + return;
> + }
> }
> partition_sysfs_add_subdir(p);
> disk->part[part-1] = p;
Well yes, but the point here it to fix kernel bugs, not to just make the
warnings go away.
The bugs here are that the effects of the kobject_add() and the
sysfs_create_link() are not undone on the error path.
Thanks for your point.
Also, please always prepare patches in `patch -p1' form, as per
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks.
Sorry. I am confused with this. Does that mean I should make patches
_upon_ the root kernel source directory or first make a copy of the
original source code and then diff against the two dirs? But I was
told that "patches should be based _in_ the root kernel source
directory" and when only one file was modified just to diff it with
the original single file. (See Documentation/SubmittingPatches.)
Can you help out? And should I remake this patch? Thanks again!
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
On Sat, 31 Mar 2007 10:30:31 +0800 "Cong WANG" <[EMAIL PROTECTED]> wrote:
> Since kobject_add, sysfs_create_link and sysfs_create_file are marked
> as '__must_check', so we must always check their return values, or gcc
> will give us warnings.
>
> Signed-off-by: Cong WANG <[EMAIL PROTECTED]>
>
> ---
> --- fs/partitions/check.c.orig2007-03-30 21:35:45.0 +0800
> +++ fs/partitions/check.c 2007-03-30 21:49:53.0 +0800
> @@ -385,10 +385,16 @@ void add_partition(struct gendisk *disk,
> p->kobj.parent = >kobj;
> p->kobj.ktype = _part;
> kobject_init(>kobj);
> - kobject_add(>kobj);
> + if (kobject_add(>kobj)) {
> + kfree(p);
> + return;
> + }
> if (!disk->part_uevent_suppress)
> kobject_uevent(>kobj, KOBJ_ADD);
> - sysfs_create_link(>kobj, _subsys.kset.kobj, "subsystem");
> + if (sysfs_create_link(>kobj, _subsys.kset.kobj, "subsystem")) {
> + kfree(p);
> + return;
> + }
> if (flags & ADDPART_FLAG_WHOLEDISK) {
> static struct attribute addpartattr = {
> .name = "whole_disk",
> @@ -396,7 +402,10 @@ void add_partition(struct gendisk *disk,
> .owner = THIS_MODULE,
> };
>
> - sysfs_create_file(>kobj, );
> + if (sysfs_create_file(>kobj, )) {
> + kfree(p);
> + return;
> + }
> }
> partition_sysfs_add_subdir(p);
> disk->part[part-1] = p;
Well yes, but the point here it to fix kernel bugs, not to just make the
warnings go away.
The bugs here are that the effects of the kobject_add() and the
sysfs_create_link() are not undone on the error path.
Also, please always prepare patches in `patch -p1' form, as per
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
On Sat, 31 Mar 2007 10:30:31 +0800 Cong WANG [EMAIL PROTECTED] wrote:
Since kobject_add, sysfs_create_link and sysfs_create_file are marked
as '__must_check', so we must always check their return values, or gcc
will give us warnings.
Signed-off-by: Cong WANG [EMAIL PROTECTED]
---
--- fs/partitions/check.c.orig2007-03-30 21:35:45.0 +0800
+++ fs/partitions/check.c 2007-03-30 21:49:53.0 +0800
@@ -385,10 +385,16 @@ void add_partition(struct gendisk *disk,
p-kobj.parent = disk-kobj;
p-kobj.ktype = ktype_part;
kobject_init(p-kobj);
- kobject_add(p-kobj);
+ if (kobject_add(p-kobj)) {
+ kfree(p);
+ return;
+ }
if (!disk-part_uevent_suppress)
kobject_uevent(p-kobj, KOBJ_ADD);
- sysfs_create_link(p-kobj, block_subsys.kset.kobj, subsystem);
+ if (sysfs_create_link(p-kobj, block_subsys.kset.kobj, subsystem)) {
+ kfree(p);
+ return;
+ }
if (flags ADDPART_FLAG_WHOLEDISK) {
static struct attribute addpartattr = {
.name = whole_disk,
@@ -396,7 +402,10 @@ void add_partition(struct gendisk *disk,
.owner = THIS_MODULE,
};
- sysfs_create_file(p-kobj, addpartattr);
+ if (sysfs_create_file(p-kobj, addpartattr)) {
+ kfree(p);
+ return;
+ }
}
partition_sysfs_add_subdir(p);
disk-part[part-1] = p;
Well yes, but the point here it to fix kernel bugs, not to just make the
warnings go away.
The bugs here are that the effects of the kobject_add() and the
sysfs_create_link() are not undone on the error path.
Also, please always prepare patches in `patch -p1' form, as per
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
2007/4/1, Andrew Morton [EMAIL PROTECTED]:
On Sat, 31 Mar 2007 10:30:31 +0800 Cong WANG [EMAIL PROTECTED] wrote:
Since kobject_add, sysfs_create_link and sysfs_create_file are marked
as '__must_check', so we must always check their return values, or gcc
will give us warnings.
Signed-off-by: Cong WANG [EMAIL PROTECTED]
---
--- fs/partitions/check.c.orig2007-03-30 21:35:45.0 +0800
+++ fs/partitions/check.c 2007-03-30 21:49:53.0 +0800
@@ -385,10 +385,16 @@ void add_partition(struct gendisk *disk,
p-kobj.parent = disk-kobj;
p-kobj.ktype = ktype_part;
kobject_init(p-kobj);
- kobject_add(p-kobj);
+ if (kobject_add(p-kobj)) {
+ kfree(p);
+ return;
+ }
if (!disk-part_uevent_suppress)
kobject_uevent(p-kobj, KOBJ_ADD);
- sysfs_create_link(p-kobj, block_subsys.kset.kobj, subsystem);
+ if (sysfs_create_link(p-kobj, block_subsys.kset.kobj, subsystem)) {
+ kfree(p);
+ return;
+ }
if (flags ADDPART_FLAG_WHOLEDISK) {
static struct attribute addpartattr = {
.name = whole_disk,
@@ -396,7 +402,10 @@ void add_partition(struct gendisk *disk,
.owner = THIS_MODULE,
};
- sysfs_create_file(p-kobj, addpartattr);
+ if (sysfs_create_file(p-kobj, addpartattr)) {
+ kfree(p);
+ return;
+ }
}
partition_sysfs_add_subdir(p);
disk-part[part-1] = p;
Well yes, but the point here it to fix kernel bugs, not to just make the
warnings go away.
The bugs here are that the effects of the kobject_add() and the
sysfs_create_link() are not undone on the error path.
Thanks for your point.
Also, please always prepare patches in `patch -p1' form, as per
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks.
Sorry. I am confused with this. Does that mean I should make patches
_upon_ the root kernel source directory or first make a copy of the
original source code and then diff against the two dirs? But I was
told that patches should be based _in_ the root kernel source
directory and when only one file was modified just to diff it with
the original single file. (See Documentation/SubmittingPatches.)
Can you help out? And should I remake this patch? Thanks again!
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
On Sun, 1 Apr 2007 14:20:46 +0800 Cong WANG [EMAIL PROTECTED] wrote: Also, please always prepare patches in `patch -p1' form, as per http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks. Sorry. I am confused with this. Does that mean I should make patches _upon_ the root kernel source directory or first make a copy of the original source code and then diff against the two dirs? But I was told that patches should be based _in_ the root kernel source directory and when only one file was modified just to diff it with the original single file. (See Documentation/SubmittingPatches.) The headers should look like: --- a/arch/cris/kernel/crisksyms.c +++ a/arch/cris/kernel/crisksyms.c I don't know how people do that. One obvious way is to do cd /usr/src diff -u linux-orig/arch/cris/kernel/crisksyms.c linux-new/arch/cris/kernel/crisksyms.c other people probably alter the diff headers. And should I remake this patch? Sure, but please change it to perform correct error handling first. And test that error handling, if you can. That will involve adding artificial errors. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [-mm3 patch]Warning fix: check the return value of kobject_add etc.
2007/4/1, Andrew Morton [EMAIL PROTECTED]: On Sun, 1 Apr 2007 14:20:46 +0800 Cong WANG [EMAIL PROTECTED] wrote: Also, please always prepare patches in `patch -p1' form, as per http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt, thanks. Sorry. I am confused with this. Does that mean I should make patches _upon_ the root kernel source directory or first make a copy of the original source code and then diff against the two dirs? But I was told that patches should be based _in_ the root kernel source directory and when only one file was modified just to diff it with the original single file. (See Documentation/SubmittingPatches.) The headers should look like: --- a/arch/cris/kernel/crisksyms.c +++ a/arch/cris/kernel/crisksyms.c I don't know how people do that. One obvious way is to do cd /usr/src diff -u linux-orig/arch/cris/kernel/crisksyms.c linux-new/arch/cris/kernel/crisksyms.c other people probably alter the diff headers. Oh, thanks. I know. And should I remake this patch? Sure, but please change it to perform correct error handling first. And test that error handling, if you can. That will involve adding artificial errors. I will remake it soon and send it again. In fact, I have just tested it roughly. And I don't how to produce artificial errors. Sorry, I hope someone can help me to test it carefully. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
