[lvm-devel] [PATCH 4/4] Update tests for lvseg apis.

[Zdenek Kabelac]

Dne 24.10.2010 23:06, Petr Rockai napsal(a):
> Zdenek Kabelac <zkabelac at redhat.com> writes:
>> I'm not going to argue whether it's more or less efficient as in this case it
>> will make no difference.  But passing things by value is simply very hardly
>> supportable by dso libraries - and usually require complete rebuild of binary
>> to use updated library. Also you would need to remember where are you using
>> passing by value - and in case structure size grows - rewrite many functions
>> to switch to pointers - why not do that right from the beginning? -  I'd pass
>> by value only the language atomic/basic types - definitely not any structure
>> where even you are pointing out future extensions in other post.
> 
> But there is a difference in adding bitfield values / union members and
> adding new members to the structure! Both mentioned extensions are
> ABI-compatible (within limits, see my other mails).
> 
>> BTW: I don't take vg_t/pv_t/lv_t as an an argument for passing things by value
>> as it's nothing else that syntactical sugar for pointers - and in fact I'd not
>> any objections against exactly same strategy for properties - making property
>> object completely private structure to lvm and give the API user only handle
>> lvm_property_t and set of function for this handle (C++ in C :))
>> (i.e.
>> lvm_property_is_string/get_string/is_integer/get_integer(lvm_property_t...)
> 
> That's both inefficient and extremely tedious to write and read. You
> have to make some compromises, C is a low level language, and by no
> means supports encapsulation or (gods forbid) OO. I will rather freeze
> the structure ABI-wise than have to write
> 
> lvm_property_t p = lvm_get_property(...)
> if (lvm_property_is_valid(p)) {
>     if (lvm_property_is_string(p)) {
>        const char *str = lvm_property_get_string(p);
>        ...
>     } else if (lvm_property_is_integer(p)) {
>        int i = lvm_property_get_integer(p);
>        ...
>     }
> }
> 

If don't see all that big difference in:

(p.is_valid)   and  (lvm_property_is_valid(p))

and as the common pattern would be to query for some known property it should
be all handled by lines like this:

if (lvm_property_get_string(p, &prop_char))

which keeps all those  'is_valid() & is_string' internal - IMHO there won't be
many tools interested in detailed diagnostic in their code....
This gives you nicer and user code that you can get with plain struct...
(IMHO everyone would write such wrapper for struct anyway...)

Exposing structures to the 'outside' world if you already think about future
extensions should be rather avoided.

> In fact, I'd rather go for C++ than for Glib, if we want to have fancy
> things in the API.

Plain C is still fancy and usable ;)

Zdenek