[pulseaudio-discuss] [patches] constification round #4 (pa_mainloop_api)

[jnqnfe at gmail.com]

On Mon, 2018-07-09 at 13:23 +0300, Tanu Kaskinen wrote:
> On Thu, 2018-07-05 at 23:51 +0100, jnqnfe at gmail.com wrote:
> > On Wed, 2018-07-04 at 11:46 +0300, Tanu Kaskinen wrote:
> > > On Mon, 2018-07-02 at 17:58 +0100, jnqnfe at gmail.com wrote:
> > > > On Fri, 2018-06-29 at 16:06 +0300, Tanu Kaskinen wrote:
> > > > > On Thu, 2018-06-28 at 04:08 +0100, jnqnfe at gmail.com wrote:
> > > > > > It's possible for patches #21, #23 and #24 to be bumped
> > > > > > down
> > > > > > before
> > > > > > the
> > > > > > API change of #20, but as things are they are blocked by it
> > > > > > and
> > > > > > it
> > > > > > would require a temporary hack... #23 and #24 (utils)
> > > > > > depend
> > > > > > upon
> > > > > > pa_signal_init(), done in #21 which involves changing the
> > > > > > underlying
> > > > > > static; this depends upon changing pa_signal_destroy_cb_t
> > > > > > (#20),
> > > > > > because of the way pa_signal_free() works (passing that
> > > > > > static
> > > > > > api
> > > > > > ref
> > > > > > to the destroy callback which requires non-const). Getting
> > > > > > around
> > > > > > this
> > > > > > would require injecting a temporary hack into
> > > > > > pa_signal_free()
> > > > > > giving
> > > > > > alternate means of obtaining a non-const api pointer - i.e.
> > > > > > api
> > > > > > function pointer comparison would be necessary to tell us
> > > > > > which
> > > > > > mainloop is in use, to then get what we need using the
> > > > > > correct
> > > > > > *_get_api() with api->userdata.
> > > > > 
> > > > > We can't change the signatures of publicly declared callback
> > > > > types.
> > > > > That will cause incompatibilities (compiler warnings at
> > > > > least,
> > > > > which
> > > > > is
> > > > > bad enough) in applications.
> > > > 
> > > > Sure
> > > > 
> > > > > Therefore the only patch that I could
> > > > > apply is the first one.
> > > > 
> > > > But the types aren't changed until #19/26 ...
> > > 
> > > Patch 4 changes the callback signatures.
> > 
> > Right, but only in the mainloop api vtable, which user's only read
> > values from, so I wasn't expecting that to be an issue.
> 
> Applications can also write to it, if they implement their own
> mainloops.

Ah, that use case was not mentioned in our previous discussion. I'd
asked if the api struct was immutable, or whether there was any
intention to allow users to modify it, to say hijack one or more
function implementations (i.e. a partial customised mainloop), and
you'd said that there was no such intention. That discussion left me
with the understanding that custom/partial-custom mainloops weren't a
thing.

Do you happen to know of any examples of such programs implementing
custom mainloops that create and pass around a mainloop-api vtable
populated with their own pointers?

> When I wrote my previous mail, I thought that patch 4 also changes
> the event callback types (pa_io_event_cb_t etc.), and that was what I
> was primarily concerned about

Yeah I deliberately left change to those callback signatures until a
later commit (#25 of 26 I believe), for precisely those same concerns.

> (but applications implementing their own mainloops is still a real
> problem).

Right... :/
Well, I guess no matter how rare custom mainloops might be, if custom
mainloops are a legit supported feature, then this change breaks it and
thus must be considered API breakage, and that holds back effectively
the entire patch set until you're willing to issue a new API-break
release :/

> I can't now find the patch that changes the callback types, but there
> are many patches that change the callback implementation signatures,
> and if there's no corresponding change to the callback types, that
> causes warnings (for example, try applying patch 2 alone, and you'll
> get warnings when building pulseaudio).