Re: question about exception throwing and ped_device_write.

[Sven Luther]

On Tue, Nov 04, 2003 at 11:16:32PM +1100, Andrew Clausen wrote:
> On Tue, Nov 04, 2003 at 01:00:46PM +0100, Sven Luther wrote:
> > > 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.
> 
> I guess so... as long as you don't ask the user 10 times to ignore.

I ask each time the block is read, which is only when reading the
partition table block and each individual partition blocks. But due to
the design of parted, it will be getting asked each time an operation is
done which will read the partition table.

> > > > 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.
> 
> It isn't unusable though, is it?

Well, i first write the partition blocks and later the partition table
block. If writing fails during one of these writing, the chained list is
broken, and i either abort or write the partition table block. If i
abort, the partition table block will point to the older partition table
chained list, and potentially bogus information (not in practice though,
since i start writing the new partition table at the same place as the
old, but still). If i write the partition table, the linked list will be
the new partitions at first, and then either a dangling link (which i
know how to detect, even loops will not stop me) or a link to the old
partition table.

I would consider this state of thing as unusable though. Maybe i will
implement a doubly linked list scheme though, so i don't mess with the
old partitions until i finish writing the new one and then only
overwrite the partition table with linking info in the new one, but this
makes for ugly organization in memory when looking at the stuff by hand.

> > 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.
> 
> No.  ped_device_write() will tell the user.

A, ok.

> > I don't like that, the user deserves to know when something he wants
> > to do failed.
> 
> Agreed.
> 
> > 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 ?
> 
> I don't think so.  I think "cancel" tells the user that the operation
> is not going to finish... that Parted has no way of working around it.

That was what i though also, but i had doubts.

> > > 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.
> 
> I should have written:
> 
>               if (ped_exception_throw() == PED_EXCEPTION_FIX)
>                       ...
> 
> Anyway, I think a switch statement is very verbose.

Sure, but you can copy&paste it and make easy modifications elsewhere.

Anyway, i will see how i can tidy up that code.

BTW, i implemented partition table reading for the filesystem code. I
still think it is ugly, but my previous method was breaking binary
compatibility, so ...

Friendly,

Sven Luther