[lvm-devel] [PATCH] (7/11) API improvements

[Dave Wysochanski]

On Mon, 2008-11-24 at 16:56 +0100, Petr Rockai wrote:
> Dave Wysochanski <dwysocha at redhat.com> writes:
> > Although the comment explains the vg_exists() function may be just a
> > guess, it is misleading to have a function called "vg_exists()" that
> > takes a vg_t * and returns true of vg_t * is NULL.
> 
> It might be counterintuitive, but that's one of the reasons the function needs
> to exist. It's pretty likely that if people would try to implement that check
> themselves, they'd get it wrong.
> 
> > Also from what I can tell, all places that call vg_exists() is preceeded
> > immediately by a call to vg_read_error() which does the same check.
> 
> Yes, but you can't rely on that when designing an API. Requiring that
> vg_read_error() is checked before using vg_exists() is counterintuitive as
> well, and counterintuitive API is worse than counterintuitive implementation of
> certain details in it. Or so I think.
> 
You are right there - it is not good to assume API call sequence for
correctness.


> So if it was me, I'd keep that interface...
> 

Ok, let me think some more about this and the error code stuff.