Re: question about exception throwing and ped_device_write.

[Sven Luther]

On Tue, Nov 04, 2003 at 10:46:28PM +1100, Andrew Clausen wrote:
> On Tue, Nov 04, 2003 at 11:47:32AM +0100, Sven Luther wrote:
> > Hello, ...
> > 
> > In the process of adding amiga support to parted i came to write the
> > following construct :
> > 
> >         if (_amiga_checksum (blk) != 0) {
> >                 switch (ped_exception_throw(PED_EXCEPTION_ERROR,
> >                         PED_EXCEPTION_FIX | PED_EXCEPTION_IGNORE |
> >                     PED_EXCEPTION_CANCEL,
> >                         _("%s : Bad checksum on block %d of type %s\n"),
> >                         __func__, block,
> >                     _amiga_block_id(blk->amiga_ID)))
> >                 {
> >                         case PED_EXCEPTION_CANCEL :
> >                                 return NULL;
> >                         case PED_EXCEPTION_FIX :
> >                                 _amiga_calculate_checksum(AMIGA(blk));
> >                                 ped_device_write (dev, blk, block, 1);
> >                         case PED_EXCEPTION_IGNORE :
> >                         case PED_EXCEPTION_UNHANDLED :
> >                         default :
> >                                 return blk;
> >                 }
> >         }
> > 
> > So, i check the checksum of the block, and ask if the checksum should be
> > fixed if it is wrong.
> 
> I don't think you should offer the option of "Ignore"... either "Fix"
> or "Cancel".  There is no reason not to fix, and you'll probably
> end up asking the user a dozen times.

Ignore is nice in case of not wanting to touch the media, but still go
ahead to see if the resulting partition table makes sense or something
such. Ideally there would be an option to dump the block or something
such to make a more informed decision on how to answer.

> > What am i supposed to do if the ped_device_write (dev, blk, block, 1);
> > fails ? In this case something is seriously wrong, and i should raise a
> > PED_EXCEPTION_FATAL ?
> 
> Fatal is when Parted destroyed something, or similar, IMHO.

Ah, ok, so i could use it for when i fail to write back some of the partition
blocks (containing the individual partition entries). In this case, the
partition table is messed up.

That said, failing to write back the individual blocks we read should
hardly happen.

> It's just an error... I'd do:
> 
>       if (!ped_device_write (dev, blk, block, 1))
>               return 0;

And thus failing silently. I don't like that, the user deserves to know
when something he wants to do failed.

Another question in the same vein. When some action failed (like
allocating memory for the blocks, reading or writing them), i throw an
exception with only the information about this in it, and provide only
the Cancel option. Would it not make better sense to provide only the Ok
or something such option instead ?

> In summary, I think you should structure this as:
> 
>       if (checksum(blk)) {
>               if (ped_exception_throw(..., "Doesn't match")) {
>                       add_checksum(blk);
>                       if (!ped_device_write(..., blk))
>                               return 0;
>               }
>       }

Mmm, i still prefer the explicit return value handling.

Friendly,

Sven Luther