lkml.org 
[lkml]   [2014]   [Sep]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 13/14] tools lib fd array: Allow associating an integer cookie with each entry
Em Thu, Sep 11, 2014 at 12:33:19PM +0200, Jiri Olsa escreveu:
> On Wed, Sep 10, 2014 at 11:08:48AM -0300, Arnaldo Carvalho de Melo wrote:

SNIP

> > @@ -80,11 +94,17 @@ int fdarray__filter(struct fdarray *fda, short revents)
> > for (fd = 0; fd < fda->nr; ++fd) {
> > - if (fda->entries[fd].revents & revents)
> > + if (fda->entries[fd].revents & revents) {
> > + if (entry_destructor)
> > + entry_destructor(fda, fd);
>
> the desctructor callback could have the 'priv' as an argument
> so we dont need to touch fdarray internals in upper layer

I thought about that, but then I would have to pass as well the pollfd
entry, that is not in priv, i.e. the callback may want to know exactly
what were the POLL{RD,WR,HUP,ER) in fdarray->entryes[fd].revents.

Yeah, sometimes it is good to avoid users touching the internals, but
that doesn't need to be something set in stone, i.e. we can consider
fda->revents and priv->priv to "public" :-)

SNIP

> > +++ b/tools/lib/api/fd/array.h
> > @@ -10,6 +10,7 @@ struct fdarray {
> > struct pollfd *entries;
> > + int *priv;

> I like the idea of private pointer for each fd, but I think it should
> be 'void*' to keep the library generic. The 'int*' is related only to
> the evlist usage of this.

Right, from the changelog comment for this patch:

------------------------------
We may need to have further info associated with each fdarray entry,
in that case we'll make that int array a 'union fdarray_priv' one and
put a pointer there so that users can stash whatever they want there.
For now, an int is enough tho.
------------------------------
I had it as:

struct fdarray {
...
union {
int idx;
void *ptr;
} priv;
...
}
But then I had no use whatsoever for the ->ptr one at this point, so I
just nuked it, to keep _just_ what is used _right now_, and added the
comment to the changelog :-)

Heck, even the comment mentions "union fdarray_priv", because at one
point I was passing that to the destructor, as you suggested, but then I
removed it to an unnamed union, because passing just the index would
allow access to ->priv[] and ->entries[].

- Arnaldo


\
 
 \ /
  Last update: 2014-09-11 16:01    [W:0.275 / U:0.332 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site