[openbeosstorage] Re: Compatibility Policy

On Wed, 24 Apr 2002, Tyler Dauwalder wrote:

> > BDirectory::Contains(): If the directory is uninitialized, the const char*
> > version taking a path, returns true, if the entry exists, whereas the
> > BEntry* version always returns false.
> > Being a lazy programmer I of course implemented one version using the
> > other, so that now both versions return true in this case.
> > I vaguely remember that there where other methods, of which different
> > versions returned different error codes for the same error.
> 
> That's kind of a tough one. I think those functions should have returned a 
> status_t: B_OK if it exists, B_ENTRY_NOT_FOUND if it doesn't, and B_NO_INIT 
> if unitialized. 
> 
> Doing what the R5 version does is safe. That's probably more important for us 
> currently than having them standardized (though it *is* important for that 
> behavior to be described in our doxygen documentation, as it is).

Alright. I wouldn't invest too much time to match and document this
behavior before the actual kernel interface is in place, though. Since, as
we agreed earlier, a lot of return values are just passed through from the
kernel.

> Then, for post-R1 I think we should replace (or augment) them with versions 
> that return status_t's. What do you think?

I general I wouldn't turn boolean methods into status_t ones, if there
isn't a good reason for it. I mean those methods implement predicates and
they are quite handy to be used this way. OTOH methods that use a boolean
return value to indicate success/failure are usually good candidates for
such a change.

> > > 1. Error codes with a functional meaning. An example of this would be
> > > BEntry::GetParent() returning B_ENTRY_NOT_FOUND when the entry is "/".
> > > B_ENTRY_NOT_FOUND *needs* to be returned in this instance. Further, any
> > > other error *needs* to return something *other* than B_ENTRY_NOT_FOUND.
> >
> > Do I understand you correctly, that you are saying, that if R5 violates
> > the rule for 1, OBOS has to do it differently (i.e. correctly)?
> 
> No. Error codes that are functionally significant should match the R5 result. 
> BEntry::GetParent() *needs* to return B_ENTRY_NOT_FOUND if the entry is the 
> root directory.

Alright.

> > * What value to return, if on an uninitialized object a method is invoked 
> >   that requires the object to be properly initialized? The InitCheck()
> >   return value, B_NO_INIT or B_FILE_ERROR?
> 
> You're right, it would be nice for this to be standardized (B_FILE_ERROR 
> should not be returned, IMO, but IIRC R5 does this in a number of cases), but 
> there are also times where something other than B_NO_INIT should be returned 
> (e.g. after trying to create a BNode on a locked file, InitCheck() should 
> return B_BUSY).

InitCheck() should always return the error code of the last SetTo(). The
interesting cases are all the other methods. And a lot of R5 methods 
behave quite consistently, returning B_FILE_ERROR, if the object is not
properly initialized.
Personally I think, that is quite reasonable.

> > * In which order shall possible errors be checked? First arguments, then
> >   InitCheck() or the other way around?
> 
> Pick one and let me know :-). I can't say that really it matters to me, but 
> I'm all for being consistent.

Fine, it seems, that in most of my code I first checked the arguments. :-)

> > * If a method invoked on an object, is supposed to initialize another
> >   object that is passed as an argument, what happens to the object, if the
> >   method fails? Unset() it at the end of the method, if an error occured
> >   or always Unset() it at the beginning of the method and leave it in the
> >   state it happens to be at the end of the method.
> 
> I like Unset()ing it at the beginning because it's easy to implement (so it's 
> easy to be consistent). Can you think of an example where this wouldn't be a 
> good idea (I can't at the moment...I need to go to bed though ;-).

:-)

Yep, I can: BPath::SetTo() allows passing BPath::Path() of the same
object. But I think, that is a special case. I bet, the same doesn't work
for BDirectory::SetTo(const BDirectory*, const char*).
So in principle I agree.

> > PS: I don't know, if I already mentioned it, BEntry doesn't handle too
> >     long entry names and cyclic links correctly. Please tell me, when you
> >     have fixed this. Then I can uncomment some of the BDirectory test
> >     cases.
> 
> Yes I know, you've mentioned it :-). I'm sorry, I really haven't had time to 
> do *anything* this quarter. I'm lucky to be staying on top of my admin 
> duties. Due to my lack of availability, you are hereby completely welcome to 
> fix anything of mine that has frustrated you to the point of being motivated 
> enough to fix it yourself. Otherwise, I shall get to it as soon as I can 
> (I'll even put off polishing up my doxygen stuff, which I'm finally getting 
> the hang of :-).

As I wrote last night, I think I'll start working on the BEntry tests and
fixing every BEntry problem I encounter. I guess, I will have finished 
this tomorrow at the latest, so that I can start with reverse engineering  
BResources then.

CU, Ingo

Other related posts: