linux-kernel - Re: [PATCH 13/14] tools lib fd array: Allow associating an integer cookie with each entry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 11 Sep 2014 10:29:11 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Jiri Olsa <jolsa@...hat.com>
Cc:	linux-kernel@...r.kernel.org,
	Adrian Hunter <adrian.hunter@...el.com>,
	Borislav Petkov <bp@...e.de>,
	Corey Ashford <cjashfor@...ux.vnet.ibm.com>,
	David Ahern <dsahern@...il.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Ingo Molnar <mingo@...nel.org>,
	Jean Pihet <jean.pihet@...aro.org>,
	Jiri Olsa <jolsa@...nel.org>,
	Namhyung Kim <namhyung@...nel.org>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/