[PATCH] Add a new wl_proxy_set_listener function

[Pekka Paalanen]

On Thu, 16 Jun 2016 14:33:44 -0400
Jonas Ådahl <jadahl at gmail.com> wrote:

> On Thu, Jun 16, 2016 at 11:27:31AM -0500, Derek Foreman wrote:
> > On 19/03/16 11:14 AM, Giulio Camuffo wrote:  
> > > The wl_proxy_add_listener function is ill-named, because it suggests
> > > that a proxy can have many listeners. Instead it can only have one so
> > > deprecate the old function and add a new wl_proxy_set_listener. The
> > > implementation of the new function is exactly the same implementation
> > > wl_proxy_add_listener had, and that is now implemented by calling
> > > wl_proxy_set_listener.
> > > Also make the scanner output new <interface>_set_listener functions.  
> > 
> > I like this, and it's
> > Reviewed-by: Derek Foreman <derekf at osg.samsung.com>
> > 
> > However, since set_listener is new, this is a chance to make
> > proxy_set_listener() bail more aggressively (perhaps abort) on a second
> > set (while still letting add_listener continue to work as it always has)
> > 
> > Do we want to do that now before it's too late?  I have no strong
> > opinion either way, so I'll defer to your judgment here.  
> 
> Do we really want to? What if you want to switch listener for whatever
> reason? Is there any reason we wouldn't want to allow it?
> 
> Given that we change the name to "set", and if we make it clear that
> setting a listener replaces the previous one, it should be obvious
> enough that it is indeed simply replaced, I don't see any reason for not
> allowing it.

That's one way to go with it, sure.

If we look at user_data, add/set_listener is not the only one setting
it, there is also set_user_data.

It seems to be a common confusion to call both set_user_data and
add_listener. If you pass different pointers to each without realizing
they both set the same member, you would be in for a surprise. Do we
really want to hold the users' hand so much to try to detect that? I
think good API design principles say we have already lost that case.

It is not unthinkable to want to re-set user_data, a little less so to
re-set the listener, but still no solid reason to forbid either.

What is the path of least surprise? I believe that is to not forbid
re-setting, so I'm with Jonas on this.

My actual patch review will be in another email.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20160617/57958ba0/attachment.sig>