Re: Fwd: C++ libubus wrapper
> For me, putting void * pointers everywhere is an anti-pattern which I
> discourage with the way the ubus api is set up. It is a lazy way to
> avoid making the code more type safe, and it adds a bit of unnecessary
> bloat to data structures.
Void pointer has one advantage i.e. it is portable and easier to use.
However, I agree that it increases memory usage so this might be a blocker.
> The ubus API is designed in a way that you typically embed the struct
> ubus_object into your own object-like data structure.
> Simple pseudo-code example:
>
> struct my_obj {
> ...
> struct ubus_object ubus;
> ...
> };
>
>
> static int
> my_obj_test(struct ubus_context *ctx, struct ubus_object *obj,
> struct ubus_request_data *req, const char *method,
> struct blob_attr *msg)
> {
> struct my_obj *obj = container_of(obj, struct my_obj, ubus);
>
> ...
> }
>
> static struct ubus_method my_obj_methods[] = {
> { .name = "test", .handle = my_obj_test },
> };
>
> As you can see, this pattern preserves some type-safety when going from
> struct ubus_object to struct my_obj and it does not waste space for an
> extra pointer. It is also more efficient, since the compiler will turn
> it into simple pointer arithmetic instead of an extra memory access.
>
> I think it would be very useful if your C++ wrapper could preserve this
> pattern and make it just as convenient to use.
>
> Maybe a C++ wrapper around ubus_object could simply be embedded as a
> class attribute in user-defined classes and you could write some code to
> generate thin wrappers that derive the object pointer from the address
> of the struct ubus_object pointer and call plain C++ methods with a
> fixed type.
>
> I don't know enough C++ to come up with an example of what that wrapper
> would look like, but it seems to me that if C can do this, it should be
> possible in C++ as well.
>
> Does that make sense?
>
> - Felix
Yes it does and as I write I think it will work. This pattern might be
not intuitive for C++
programmers but it is fine with me.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Re: Fwd: C++ libubus wrapper
On 2021-02-04 18:25, Wojciech Jowsa wrote:
> czw., 4 lut 2021 o 10:32 Felix Fietkau napisał(a):
>>
>>
>> On 2021-02-03 20:02, Wojciech Jowsa wrote:
>> > Hi,
>> >
>> > I would like to write a libubus wrapper in C++. When looking into
>> > ubus_method and ubus_handler_t structures I don't see a place where I
>> > could pass a this pointer. It makes the handling of ubus calls in C++
>> > a bit complicated.
>> > The solution to that case would be adding a void pointer to the
>> > ubus_method structure (like in the ubus_request struct). This way it
>> > would be possible to pass e.g function pointer.
>> > Would it be possible to add a void pointer to ubus_method structure?
>> > If yes then I will provide the patch asap.
>> I don't think a 'this' pointer should be in the ubus_method. Methods can
>> typically be shared between multiple ubus_objects, and the 'this'
>> pointer would typically go into the ubus_object.
>
> The pointer would share the same concept as the .handler member of
> ubus_method structure.
> It means that C++ handler method would be called based on the ubus method
> name.
> Void pointer could also be the last argument of ubus_handler_t. Then
> I see that e.g. ubus_request structure has a void pointer as a member
> or ubus_lookup has a void pointer
> parameter so the idea is implemented but only partially.
The idea has been implemented selectively where it makes sense.
In the ubus request it is needed, because functions like ubus_invoke()
create it on-stack, and there is no embedding into other data structures.
>> However, even there it is not necessary to have a void pointer directly
>> in the struct. You can simply embed the ubus_object in another data
>> structure which contains the 'this' pointer (or maybe is even embedded
>> in the C++ object directly)
>
> Then obtain parent struct with container_of? This might somehow work
> but personally I have never tried that.
> I think that the common pattern for C libraries API methods is to
> allow passing a user data through a void pointer either as
> a function parameter or as a structure member.
For me, putting void * pointers everywhere is an anti-pattern which I
discourage with the way the ubus api is set up. It is a lazy way to
avoid making the code more type safe, and it adds a bit of unnecessary
bloat to data structures.
The ubus API is designed in a way that you typically embed the struct
ubus_object into your own object-like data structure.
Simple pseudo-code example:
struct my_obj {
...
struct ubus_object ubus;
...
};
static int
my_obj_test(struct ubus_context *ctx, struct ubus_object *obj,
struct ubus_request_data *req, const char *method,
struct blob_attr *msg)
{
struct my_obj *obj = container_of(obj, struct my_obj, ubus);
...
}
static struct ubus_method my_obj_methods[] = {
{ .name = "test", .handle = my_obj_test },
};
As you can see, this pattern preserves some type-safety when going from
struct ubus_object to struct my_obj and it does not waste space for an
extra pointer. It is also more efficient, since the compiler will turn
it into simple pointer arithmetic instead of an extra memory access.
I think it would be very useful if your C++ wrapper could preserve this
pattern and make it just as convenient to use.
Maybe a C++ wrapper around ubus_object could simply be embedded as a
class attribute in user-defined classes and you could write some code to
generate thin wrappers that derive the object pointer from the address
of the struct ubus_object pointer and call plain C++ methods with a
fixed type.
I don't know enough C++ to come up with an example of what that wrapper
would look like, but it seems to me that if C can do this, it should be
possible in C++ as well.
Does that make sense?
- Felix
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Fwd: C++ libubus wrapper
czw., 4 lut 2021 o 10:32 Felix Fietkau napisał(a):
>
>
> On 2021-02-03 20:02, Wojciech Jowsa wrote:
> > Hi,
> >
> > I would like to write a libubus wrapper in C++. When looking into
> > ubus_method and ubus_handler_t structures I don't see a place where I
> > could pass a this pointer. It makes the handling of ubus calls in C++
> > a bit complicated.
> > The solution to that case would be adding a void pointer to the
> > ubus_method structure (like in the ubus_request struct). This way it
> > would be possible to pass e.g function pointer.
> > Would it be possible to add a void pointer to ubus_method structure?
> > If yes then I will provide the patch asap.
> I don't think a 'this' pointer should be in the ubus_method. Methods can
> typically be shared between multiple ubus_objects, and the 'this'
> pointer would typically go into the ubus_object.
The pointer would share the same concept as the .handler member of
ubus_method structure.
It means that C++ handler method would be called based on the ubus method name.
Void pointer could also be the last argument of ubus_handler_t. Then
I see that e.g. ubus_request structure has a void pointer as a member
or ubus_lookup has a void pointer
parameter so the idea is implemented but only partially.
> However, even there it is not necessary to have a void pointer directly
> in the struct. You can simply embed the ubus_object in another data
> structure which contains the 'this' pointer (or maybe is even embedded
> in the C++ object directly)
Then obtain parent struct with container_of? This might somehow work
but personally I have never tried that.
I think that the common pattern for C libraries API methods is to
allow passing a user data through a void pointer either as
a function parameter or as a structure member.
> I don't write code in C++ myself, so if there's something I'm missing
> here, please let me know and show me some details.
The flow would be following (I omit external event-loop handling)
//Define function pointer type and a struct which contains a function pointer.
//The function pointer is the same as ubus_handler_t
//The struct is required to be able to cast to void*
typedef std::function IUbus_f_ptr;
struct IUbusEntry
{
IUbus_f_ptr f_ptr;
};
//In the class using ubus wrapper, bind member function (handler) with
this and call addUbusObject method from C++ ubus wrapper
int ubusCB(ubus_context *ctx, struct ubus_object *obj, struct
ubus_request_data *req, const char *method, struct blob_attr *msg);
IUbus::IUbusEntry_t mUbus_cb;
mUbus_cb.f_ptr = std::bind(&UBusHandler::ubusCB, this, _1, _2, _3, _4, _5);
mUbus.addObject(&mUbus_cb);
// In addObject(IUbusEntry_t *aCallback), set .priv to aCallback method.
// In a static testMethod match a method based on its name and call a
C++ callback (priv pointer).
ubus_method lMethod;
memset(&lMethod, 0, sizeof(ubus_method));
lMethod.name = "restart";
lMethod.handler = &IUbus::testMethod;
lMethod.priv = (void*) aCallback;
int IUbus::testMethod( ubus_context *ctx, ubus_object *obj,
ubus_request_data *req, const char *method,
blob_attr *msg)
{
for(int i = 0; i < obj->n_methods; i++) {
if(!strcmp(obj->methods[i].name, method)) {
return ((IUbus::IUbusEntry_t
*)(obj->methods[i].priv))->f_ptr(ctx, obj, req, method, msg);
}
}
return 0;
}
As I wrote before the void pointer could be a part of ubus_method
struct or the last argument of ubus_handler_t.
It would really make using ubus in C++ projects easier :)
BR,
Wojtek
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
