2004-09-15 11:18:20

by Pavel Machek

Subject: Untangle code in bio.c

Hi!

bio.c uses quite ugly code with goto's, completely
unneccessarily. Please apply,
Pavel

--- clean-mm/fs/bio.c 2004-09-15 12:58:10.000000000 +0200
+++ linux-mm/fs/bio.c 2004-09-15 13:00:51.000000000 +0200
@@ -143,7 +143,7 @@

bio = mempool_alloc(bio_pool, gfp_mask);
if (unlikely(!bio))
- goto out;
+ return NULL;

bio_init(bio);

@@ -157,13 +157,11 @@
noiovec:
bio->bi_io_vec = bvl;
bio->bi_destructor = bio_destructor;
-out:
return bio;
}

mempool_free(bio, bio_pool);
- bio = NULL;
- goto out;
+ return NULL;
}

/**
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

2004-09-15 11:38:51

by Andrew Morton

Subject: Re: Untangle code in bio.c

Pavel Machek <[email protected]> wrote:
>
> Hi!
>
> bio.c uses quite ugly code with goto's, completely
> unneccessarily. Please apply,

I wouldn't describe this as an improvement, really. Multiple return
statements give me hysterics.

>
> --- clean-mm/fs/bio.c 2004-09-15 12:58:10.000000000 +0200
> +++ linux-mm/fs/bio.c 2004-09-15 13:00:51.000000000 +0200
> @@ -143,7 +143,7 @@
>
> bio = mempool_alloc(bio_pool, gfp_mask);
> if (unlikely(!bio))
> - goto out;
> + return NULL;
>
> bio_init(bio);
>
> @@ -157,13 +157,11 @@
> noiovec:
> bio->bi_io_vec = bvl;
> bio->bi_destructor = bio_destructor;
> -out:
> return bio;
> }
>
> mempool_free(bio, bio_pool);
> - bio = NULL;
> - goto out;
> + return NULL;
> }

How's this look?

struct bio *bio_alloc(int gfp_mask, int nr_iovecs)
{
struct bio *bio = mempool_alloc(bio_pool, gfp_mask);

if (likely(bio)) {
struct bio_vec *bvl = NULL;

bio_init(bio);
if (likely(nr_iovecs)) {
unsigned long idx;

bvl = bvec_alloc(gfp_mask, nr_iovecs, &idx);
if (unlikely(!bvl)) {
mempool_free(bio, bio_pool);
bio = NULL;
goto out;
}
bio->bi_flags |= idx << BIO_POOL_OFFSET;
}
bio->bi_io_vec = bvl;
bio->bi_destructor = bio_destructor;
}
out:
return bio;
}

2004-09-15 11:57:36

by Jens Axboe

Subject: Re: Untangle code in bio.c

On Wed, Sep 15 2004, Andrew Morton wrote:
> Pavel Machek <[email protected]> wrote:
> >
> > Hi!
> >
> > bio.c uses quite ugly code with goto's, completely
> > unneccessarily. Please apply,
>
> I wouldn't describe this as an improvement, really. Multiple return
> statements give me hysterics.

Me neither...

> > --- clean-mm/fs/bio.c 2004-09-15 12:58:10.000000000 +0200
> > +++ linux-mm/fs/bio.c 2004-09-15 13:00:51.000000000 +0200
> > @@ -143,7 +143,7 @@
> >
> > bio = mempool_alloc(bio_pool, gfp_mask);
> > if (unlikely(!bio))
> > - goto out;
> > + return NULL;
> >
> > bio_init(bio);
> >
> > @@ -157,13 +157,11 @@
> > noiovec:
> > bio->bi_io_vec = bvl;
> > bio->bi_destructor = bio_destructor;
> > -out:
> > return bio;
> > }
> >
> > mempool_free(bio, bio_pool);
> > - bio = NULL;
> > - goto out;
> > + return NULL;
> > }
>
> How's this look?
>
> struct bio *bio_alloc(int gfp_mask, int nr_iovecs)
> {
> struct bio *bio = mempool_alloc(bio_pool, gfp_mask);
>
> if (likely(bio)) {
> struct bio_vec *bvl = NULL;
>
> bio_init(bio);
> if (likely(nr_iovecs)) {
> unsigned long idx;
>
> bvl = bvec_alloc(gfp_mask, nr_iovecs, &idx);
> if (unlikely(!bvl)) {
> mempool_free(bio, bio_pool);
> bio = NULL;
> goto out;
> }
> bio->bi_flags |= idx << BIO_POOL_OFFSET;
> }
> bio->bi_io_vec = bvl;
> bio->bi_destructor = bio_destructor;
> }
> out:
> return bio;
> }

Same semantics and it looks good to me.

--
Jens Axboe