[PATCH wayland] server: add helper functions for wl_global

[Pekka Paalanen]

On Mon, 9 Mar 2015 01:51:27 -0700
Bryce Harrington <bryce at osg.samsung.com> wrote:

> On Mon, Mar 09, 2015 at 09:49:19AM +0200, Pekka Paalanen wrote:
> > On Mon, 23 Feb 2015 17:20:37 -0800
> > Bryce Harrington <bryce at osg.samsung.com> wrote:
> > 
> > > On Mon, Feb 23, 2015 at 11:30:57AM -0800, Bill Spitzak wrote:
> > > > Which of the 4 arguments do you use for your use case?
> > > > 
> > > > Because this can only return the first match I suspect some of the
> > > > NULL tests are pretty useless, because you have to specify arguments
> > > > that you know will return the one and only instance you are
> > > > interested in. It might be better to make this exactly match the use
> > > > case you have. If you are passing NULL just remove that argument,
> > > > and conversely remove the null test from this code for the arguments
> > > > you send. Basically don't add code that is not certain to be useful.
> > > 
> > > I would assume he's using all of them.
> > > 
> > > Regardless, so long as the test case covers an adequate number of
> > > permutations I don't see an issue with providing a generalized solution
> > > to this need.
> > > 
> > > However, I suspect this API would be better named as
> > > wl_global_find_first().
> > > 
> > > A more general wl_global_find() I would expect to be returning a list of
> > > results.
> 
> Lacking some sort of 'yield' like mechanism, wouldn't returning a list
> incur performance impacts?

You'd go from average O(N/2) to O(N), which are both equivalent to O(N)
- sorry if my terminology is off, but I hope it shows the point.

The API could be something like:
int wl_global_find(size_t N, struct wl_global **arr, ...<conditions>)

where N is the number of elements available to be written in array arr,
and the return value is how many elements would have been written if arr
was large enough. Arr could be NULL to just count the number of hits.
Arr would get written up to N globals.

So in the worst case we'd have O(2N) for first counting the globals and
then returning them, but usually you expect some small number like 1
max, so you could do with just one call.

What performance concerns do you have?


Thanks,
pq

> > > One thing to think about when writing a test case, is if this function
> > > will always return the same first result.  E.g. if the globals list
> > > depends at all on timing of hardware bringup thight might vary boot to
> > > boot, or if hashes are in any way involved in creating it, or etc.
> > > I'm guessing that's not the case, but probably worth checking, else
> > > we'll inevitably have a bug reported about it on some obscure piece of
> > > hardware.
> > 
> > Hi,
> > 
> > I'm certainly in favour of making this "find" API to return *all*
> > matches, not just a random match like it is now. Then, if the caller
> > expects to ever find either 0 or 1 globals, it can yell if that
> > invariant is ever broken.

...

> > > > On 02/23/2015 10:06 AM, Jonny Lamb wrote:
> > > > 
> > > > >+/** Find a wl_global given search parameters
> > > > >+ *
> > > > >+ * \param display The display object
> > > > >+ * \param interface The interface to match, or NULL
> > > > >+ * \param version The version to match, or 0
> > > > >+ * \param data The user data to match, or NULL
> > > > >+ * \param bind The function used to bind the object, or NULL
> > > > >+ * \return The first wl_global which matches, or NULL
> > > > >+ *
> > > > >+ * The global list of a wl_display is not public so this function helps in
> > > > >+ * finding a specific wl_global based on the parameters. Parameters can be
> > > > >+ * missing for a broader search (NULL for interface, data, and bind, and 0 for
> > > > >+ * version) but at least one must be present or no result will be returned.
> > > > >+ *
> > > > >+ * \memberof wl_global
> > > > >+ */
> > > > >+WL_EXPORT struct wl_global *
> > > > >+wl_global_find(struct wl_display *display,
> > > > >+	const struct wl_interface *interface, uint32_t version,
> > > > >+	void *data, wl_global_bind_func_t bind)