[openbeosstorage] Re: DiskDevice API v2.4

Tyler Dauwalder <tyler@xxxxxxxxxxxxx> wrote:
> > Yep, just go ahead. I'm looking forward to seeing an updated 
> > version. 
> > :-P
> 
> Done. :-)
> 
> http://www.dauwalder.net/DiskDeviceAPI_v2.4.zip

Nice. Thanks.

> A few notes:
> 
> - I discovered and thus added in DiskDevicePrivate.h from 
> headers/private/storage after updating it.

OK, we'll see how much we'll need of it.

> - I changed all int32s to uint32s, since, as Axel mentioned, Amiga 
> can 
> have some 4 billion partitions technically speaking, I figured there 
> was 
> no reason to skimp on bits due to laziness (which I assume is why all 
> the int32s showed up so far; they didn't appear to be needed anywhere 
> to 
> allow for negative return values).

Mmh, for the IDs I could probably live with uint32 -- although I liked 
to use -1 as an invalid ID (in case of CommitModifications() we might 
even want to have that to indicate an error) -- but for indices and 
counts, I'd strongly vote for int32. That's what's used all over the 
place in the BeOS API (there may be a few exceptions regarding counts, 
but I don't think more than two for indices (the private window_at() 
and BScreen::[Set]DesktopColor() (vs. several hundreds int32 
occurences))) and I wouldn't like to introduce non-uniformity here.

I'm not sure, why Be decided to use signed types, but I guess to 
prevent that one can by accident easily write dangerous iteration code 
like this:

for (uint32 i = count - 1; i >= 0; i--) { ... }

(Note, two traps: count - 1 and i >= 0.)

> - Is your intent to populate a given BPartioningInfo object with data 
> once and only once upon the call to 
> BPartition::GetPartitioningInfo(), 
> forcing a refresh by getting a new object via another call to 
> BPartition::GetPartitioningInfo()? I assumed so thus far in making 
> BPartitioningInfo.

A bit different. I'd rather like to have a bool/status_t 
GetPartitioningInfo(BPartitioningInfo*) const, so that one can create a 
BPartitioningInfo on the stack and doesn't always get a newly allocated 
one. I would probably also drop BPartitioningInfo::Parent().

> - I added in all the iterations of the DiskDevice API headers to our 
> cvs, just for good measure.

Cool!

CU, Ingo

Other related posts: