Re: [KJ] [PATCH] check request_region return value

[Tim Cooijmans <tim@aapopfiets.nl>]

On Saturday 25 February 2006 15:13, Jesper Juhl wrote:
> > +out_gpe1_blk:
> > +       if (acpi_gbl_FADT->xgpe0_blk.address &&
> > acpi_gbl_FADT->gpe0_blk_len && +           !(acpi_gbl_FADT->gpe0_blk_len
> > & 0x1))
> > +               release_region(acpi_gbl_FADT->xgpe0_blk.address,
> > +                              acpi_gbl_FADT->gpe0_blk_len);
> > +out_gpe0_blk:
> > +       if (acpi_gbl_FADT->xpm2_cnt_blk.address &&
> > acpi_gbl_FADT->pm2_cnt_len) +              
> > release_region(acpi_gbl_FADT->xpm2_cnt_blk.address, +                    
> >          acpi_gbl_FADT->pm2_cnt_len);
> > +out_pm2_cnt_blk:
> > +       if (acpi_gbl_FADT->xpm_tmr_blk.address &&
> > acpi_gbl_FADT->pm_tm_len = 4) +              
> > release_region(acpi_gbl_FADT->xpm_tmr_blk.address, 4); +out_pm_tmr:
> > +       if (acpi_gbl_FADT->xpm1b_cnt_blk.address &&
> > acpi_gbl_FADT->pm1_cnt_len) +              
> > release_region(acpi_gbl_FADT->xpm1b_cnt_blk.address, +                   
> >           acpi_gbl_FADT->pm1_cnt_len);
> > +out_pm1b_cnt_blk:
> > +       if (acpi_gbl_FADT->xpm1a_cnt_blk.address &&
> > acpi_gbl_FADT->pm1_cnt_len) +              
> > release_region(acpi_gbl_FADT->xpm1a_cnt_blk.address, +                   
> >           acpi_gbl_FADT->pm1_cnt_len);
> > +out_pm1a_cnt_blk:
> > +       if (acpi_gbl_FADT->xpm1b_evt_blk.address &&
> > acpi_gbl_FADT->pm1_evt_len) +              
> > release_region(acpi_gbl_FADT->xpm1b_evt_blk.address, +                   
> >           acpi_gbl_FADT->pm1_evt_len);
> > +out_pm1b_evt_blk:
> > +       if (acpi_gbl_FADT->xpm1a_evt_blk.address &&
> > acpi_gbl_FADT->pm1_evt_len) +              
> > release_region(acpi_gbl_FADT->xpm1a_evt_blk.address, +                   
> >           acpi_gbl_FADT->pm1_evt_len);
> > +out_pm1a_evt_blk:
> > +       return -EBUSY;
> >  }
>
> Why all these if's in the error path? Since you get to any of these
> labels in the first place you already know what request_region()s
> succeeded, so what's the point of checking all these variables before
> calling release_region()?
Not all request_region()s are tried.  These checks are done before requesting 
them as well, to determine which ones should be requested.  If I don't do 
these checks here, some release_region() calls will try to release regions 
that were never requested.

I'll look into your other comments.

Thanks,
Tim
_______________________________________________
Kernel-janitors mailing list
Kernel-janitors@lists.osdl.org
https://lists.osdl.org/mailman/listinfo/kernel-janitors