#idiomatic way to wrap a c function that takes a callback

1 messages · Page 1 of 1 (latest)

spice cargo
#

Hey, so I have a c function with this signature pub extern fn mpv_set_wakeup_callback(ctx: ?*mpv_handle, cb: ?*const fn (?*anyopaque) callconv(.C) void, d: ?*anyopaque) void;, what would be the preferred way to wrap it? should I just leave as is and expect from the user to pass a callconv(.c) function?

#

currently I @ptrCast normal zig function but I read that this may result in unexpected behavior.

spice cargo
#

hmm

twilit jasper
#

Indeed, ptrcasting is a bad idea because you're asking Zig to pretend that a Zig function has a different calling convention to what it actually is.

#

But the C caller doesn't know that

#

And Zig's calling convention is not well-defined, IIUC

#

So it's allowed to be whatever it wants

#

It's fairly normal to use anyopaques to pass down user context data, so that part is fine.
Not sure how much it really matters if the user has to supply a C function or not.
If this is just in your code, I probably wouldn't bother doing anything on top of it.
If it's a zig library --- honestly, I don't feel like it really matters that much. You'd only be adding a layer of inefficiency to force the caller to pass a zig function, only to make a C function to call it. 🤔

spice cargo
#

Yes I was thinking about passing the user function into an inside function using the d: ?*anyopaque then call it from there but it

#

is just so much work

#

So I was wondering if it is worth the hassle

twilit jasper
#

Keep in mind that the user callback probably will want to pass through their own context pointer using that argument

#

So not sure you'd want to use it for that

#

But yeah - it's one of those things where I would want to remove the Cisms, but at the same time, it's often better to not add abstraction for the sake of it.

spice cargo
twilit jasper
#

Yeah - that sounds more like it

gaunt stream
twilit jasper
#

Yeah

#

Was gonna say

#

If it's literally just a single C function callback that will be used for all user callbacks, and this callback isn't going be called a load of times, (or you always-inline), then maybe it's fine(?)

#

But I ultimately don't know how much it's really worth it to do stuff like this.

gaunt stream
#

i think its worthwhile for type safety, you can accept a comptime T: type param to make it generic over the actual type of the user data

twilit jasper
#

I'd honestly probably just advice against that in favour of *anyopaque.

spice cargo
twilit jasper
#

Type safety can become a bit masterbatory if you lean on it for correctness.

#

"You MUST do X in order to Y, in order to do Z!!!"

#

It can pay to not reach for it literally everywhere that you can.

twilit jasper
#

Do note that means there'll be a C callback for every user callback, however.

gaunt stream
#

it does call your function inline tho so the C function is the only function

#

effectively no different than there being no wrapper

spice cargo
#

That function I am trying to wrap is gonna be called too much, So I don't know if it will have much performance cost!

gaunt stream
#

why is a function to set a callback called often?

spice cargo
#

Ah, sorry. This is just one of a few functions that I am try to wrap.

#

the other one is update_callback

#

so it's gonna be called way too often

twilit jasper
#

In fairness, if it's inlined into the C callback -- which it will if always_inline is used; at least that's the idea -- it should only mean that the Zig callback is copied, and thus only that C function will exist at runtime.

#

Assuming you don't call the Zig callback directly from anywhere else, of course

#

It's a bit technical, which I don't like honestly

#

But you shouldn't end up with multiple copies of the same user callback in the exe at least

#

Not that I've personally verified that, mind.

spice cargo
#

Alright, I think I will go with inline the function since It looks like it doesn't cost that much of performance.