Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
MIME-Version: 1.0
In-Reply-To: <49F9F117-9C1F-4D44-AB02-9505D7614265@sharp.fm>
References: <61123A0F-0B90-4DCD-B995-30E961DDCBF3@sharp.fm>
	<CALK=YjOnyjO0YsZ63rD-rXNPHyHE03owXpWRDP3O+Je=4iNQJg@mail.gmail.com>
	<F37BF2FE-6E64-4F39-A0E0-7BC959EE543A@sharp.fm>
	<CALK=YjM-pZysD=DC0jUC+r14BsY9HXoXriO9_NDY8mOVxYqnHg@mail.gmail.com>
	<DFE8326E-1E4F-47D4-BDA8-5ADE5DAE4161@sharp.fm>
	<CAKQ1sVPqUBzEg3aNkmbXi6wyv=FKZYPEN0iOBv_cFeVc8Wng6g@mail.gmail.com>
	<65C15DDA-BCC2-49F4-B3D7-96B4853F524E@sharp.fm>
	<CAKQ1sVOQN_t=+=cMciUvKk-LV-f75TyWUb7G8w=gAEiYfDuvnw@mail.gmail.com>
	<9A01C97A-18EF-49B4-ACE9-84C9290ACD1B@sharp.fm>
	<CAKQ1sVNzuhgL6HoG64hV71sPJp623iZTEpn6Hyvc7Lh7SNuNMg@mail.gmail.com>
	<49F9F117-9C1F-4D44-AB02-9505D7614265@sharp.fm>
Date: Tue, 15 Mar 2016 22:03:03 +0100
Message-ID: 
 <CAKQ1sVNn0YFjroStF5u2bgt3_Q0D7w9b-eVizYeV3Q7SZFrRSQ@mail.gmail.com>
Subject: Re: Proposed change to mpm_register_socket_callback(): apr_socket_t
 -> apr_pollfd_t
From: Yann Ylavic <ylavic.dev@gmail.com>
To: httpd-dev <dev@httpd.apache.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Tue, Mar 15, 2016 at 8:28 PM, Graham Leggett <minfrin@sharp.fm> wrote:
>
> The trouble with the above is that because of the pool cleanup we now hav=
e, pfds[3] needs to live as long as pool p. In your example it does, but th=
ere is nothing stopping someone trying to allocate pfds[3] on the stack and=
 then returning. Later the cleanup is run, and boom - difficult to debug cr=
ash or weird behaviour.

Was just an example on the possibily to use a plain array for the
interface (a stacked one is of course not recommended to register an
event callback likely run out of scope).

>
> With the array you=E2=80=99re guaranteed the memory is allocated from a p=
ool, which means the pool cleanup will always be safe.

You convinced me, I like your may better now ;)
Possibly we could also force each pfd->p to pfds->pool in
event_register_poll_callback_ex().

>
> What we should also do is drop the apr_pool_t *p parameter and read it fr=
om apr_header_array_t=E2=80=99s pool instead. This will be a further check =
to stop the caller from doing anything pathological, as we definitely know =
the cleanup and the array belong to each other, and our API becomes simpler=
 still.
>
> Attached patch does this.

+1

Regards,
Yann.