2005-10-17 13:22:52

by glauber

[permalink] [raw]
Subject: [PATCH] Test for sb_getblk return value

Hi all,

As we discussed earlier, I'm sending a patch that adds test for the
return value of sb_getblk. This time I focused on the code of the ext2/3
filesystems. I'm assuming that getblk fails happens due to I/O errors
and thus returning returning an EIO back wherever it's needed.

Al, hope it's better this time.

--
=====================================
Glauber de Oliveira Costa
IBM Linux Technology Center - Brazil
[email protected]
=====================================


Attachments:
(No filename) (479.00 B)
patch_getblk_ext3 (3.03 kB)
Download all attachments

2005-10-17 14:10:11

by Jesper Juhl

[permalink] [raw]
Subject: Re: [PATCH] Test for sb_getblk return value

On 10/17/05, Glauber de Oliveira Costa <[email protected]> wrote:
> Hi all,
>
> As we discussed earlier, I'm sending a patch that adds test for the
> return value of sb_getblk. This time I focused on the code of the ext2/3
> filesystems. I'm assuming that getblk fails happens due to I/O errors
> and thus returning returning an EIO back wherever it's needed.
>

> - bh = sb_getblk(inode->i_sb, parent);
> + if (!(bh = sb_getblk(inode->i_sb, parent))){
> + err = -EIO;
> + break;
> + }

Would be more readable as

bh = sb_getblk(inode->i_sb, parent);
if (!bh) {
err = -EIO;
break;
}


--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

Subject: Re: [PATCH] Test for sb_getblk return value

I'm resending it now with the changes you suggested.
Actually, 2 copies of it follows.

In the first one(v2), I kept the style in the changes in resize.c, as this
seems to be the default way things like this are done there. In the other,
(v3), I did statement checks in the way you suggested in both files.

Also, sorry for the last mail. I got a problem with my relay, and my mail
address was sent wrong before I noticed that. Mails sent to it will probably
return.

On Monday 17 October 2005 12:10, Jesper Juhl wrote:
> On 10/17/05, Glauber de Oliveira Costa <[email protected]> wrote:
> > Hi all,
> >
> > As we discussed earlier, I'm sending a patch that adds test for the
> > return value of sb_getblk. This time I focused on the code of the ext2/3
> > filesystems. I'm assuming that getblk fails happens due to I/O errors
> > and thus returning returning an EIO back wherever it's needed.
> >
> >
> > - bh = sb_getblk(inode->i_sb, parent);
> > + if (!(bh = sb_getblk(inode->i_sb, parent))){
> > + err = -EIO;
> > + break;
> > + }
>
> Would be more readable as
>
> bh = sb_getblk(inode->i_sb, parent);
> if (!bh) {
> err = -EIO;
> break;
> }
>
>
> --
> Jesper Juhl <[email protected]>
> Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
> Plain text mails only, please http://www.expita.com/nomime.html
> -
> 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/

--
=====================================
Glauber de Oliveira Costa
IBM Linux Technology Center - Brazil
[email protected]
=====================================


Attachments:
(No filename) (1.74 kB)
patch_getblk_ext3.v2 (2.80 kB)
patch_getblk_ext3.v3 (2.62 kB)
Download all attachments

2005-10-18 23:33:34

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] Test for sb_getblk return value

Glauber de Oliveira Costa <[email protected]> wrote:
>
> I'm resending it now with the changes you suggested.
> Actually, 2 copies of it follows.

argh. Please never attach multiple patches to a single email.

And please always include a complete, uptodate changelog with each iteration
of a patch. I don't want to have to troll back through the mailing list,
identify the initial changelog and then replay the email thread making any
needed updates to that changelog.

Also please review section 11 of Documentation/SubmittingPatches then
include a Signed-off-by: with your patches.

> In the first one(v2), I kept the style in the changes in resize.c, as this
> seems to be the default way things like this are done there. In the other,
> (v3), I did statement checks in the way you suggested in both files.

Don't worry about the surrounding style - if it's wrong, it's wrong. Just
stick with Documentation/CodingStyle.

Do this:

if (!bh) {

and not this:

if (!bh){

> Also, sorry for the last mail. I got a problem with my relay, and my mail
> address was sent wrong before I noticed that. Mails sent to it will probably
> return.

The change to update_backups() is wrong - it will leave a JBD transaction
open on return.

Please fix all that up and resend.
http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt may prove useful,
thanks.

Subject: Re: [PATCH] Test for sb_getblk return value

Andrew,

Thanks a lot for your patience and review. And also for the document. It
was really helpfull. Patch follows in next mail.



On Tue, Oct 18, 2005 at 04:32:01PM -0700, Andrew Morton wrote:
> Glauber de Oliveira Costa <[email protected]> wrote:
> >
> > I'm resending it now with the changes you suggested.
> > Actually, 2 copies of it follows.
>
> argh. Please never attach multiple patches to a single email.
>
> And please always include a complete, uptodate changelog with each iteration
> of a patch. I don't want to have to troll back through the mailing list,
> identify the initial changelog and then replay the email thread making any
> needed updates to that changelog.
>
> Also please review section 11 of Documentation/SubmittingPatches then
> include a Signed-off-by: with your patches.
>
> > In the first one(v2), I kept the style in the changes in resize.c, as this
> > seems to be the default way things like this are done there. In the other,
> > (v3), I did statement checks in the way you suggested in both files.
>
> Don't worry about the surrounding style - if it's wrong, it's wrong. Just
> stick with Documentation/CodingStyle.
>
> Do this:
>
> if (!bh) {
>
> and not this:
>
> if (!bh){
>
> > Also, sorry for the last mail. I got a problem with my relay, and my mail
> > address was sent wrong before I noticed that. Mails sent to it will probably
> > return.
>
> The change to update_backups() is wrong - it will leave a JBD transaction
> open on return.
>
> Please fix all that up and resend.
> http://www.zip.com.au/~akpm/linux/patches/stuff/tpp.txt may prove useful,
> thanks.
>
>

--
=====================================
Glauber de Oliveira Costa
IBM Linux Technology Center - Brazil
[email protected]
=====================================

Subject: Re: [PATCH] Test for sb_getblk return value

This patch adds tests for the return value of sb_getblk() in the ext2/3
filesystems. In fs/buffer.c it is stated that the getblk() function
never fails. However, it does can return NULL in some situations due to
I/O errors, which may lead us to NULL pointer dereferences

Signed-off-by: Glauber de Oliveira Costa <[email protected]>

--
=====================================
Glauber de Oliveira Costa
IBM Linux Technology Center - Brazil
[email protected]
=====================================


Attachments:
(No filename) (498.00 B)
patch_getblk_ext3.v4 (2.61 kB)
Download all attachments