You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Rainer Jung <ra...@kippdata.de> on 2016/08/09 21:31:17 UTC

Re: svn commit: r1736521 - in /apr/apr/branches/1.6.x: ./ include/ include/arch/unix/ poll/unix/ test/

Am 25.03.2016 um 02:19 schrieb minfrin@apache.org:
> Author: minfrin
> Date: Fri Mar 25 01:19:34 2016
> New Revision: 1736521
>
> URL: http://svn.apache.org/viewvc?rev=1736521&view=rev
> Log:
> Add apr_pollcb_wakeup(), with similar behavior to
> apr_pollset_wakeup(). Add apr_pollcb_method_name(), with similar
> behavior to apr_pollset_method_name().
>

...

> Modified: apr/apr/branches/1.6.x/poll/unix/port.c
> URL: http://svn.apache.org/viewvc/apr/apr/branches/1.6.x/poll/unix/port.c?rev=1736521&r1=1736520&r2=1736521&view=diff
> ==============================================================================
> --- apr/apr/branches/1.6.x/poll/unix/port.c (original)
> +++ apr/apr/branches/1.6.x/poll/unix/port.c Fri Mar 25 01:19:34 2016
> @@ -413,7 +413,7 @@ static apr_status_t impl_pollset_poll(ap
>              if ((pollset->flags & APR_POLLSET_WAKEABLE) &&
>                  fp.desc_type == APR_POLL_FILE &&
>                  fp.desc.f == pollset->wakeup_pipe[0]) {
> -                apr_pollset_drain_wakeup_pipe(pollset);
> +                apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe);
>                  rv = APR_EINTR;
>              }
>              else {
> @@ -466,9 +466,8 @@ static apr_pollset_provider_t impl = {
>
>  apr_pollset_provider_t *apr_pollset_provider_port = &impl;
>
> -static apr_status_t cb_cleanup(void *p_)
> +static apr_status_t impl_pollcb_cleanup(apr_pollcb_t *pollcb)
>  {
> -    apr_pollcb_t *pollcb = (apr_pollcb_t *) p_;
>      close(pollcb->fd);
>      return APR_SUCCESS;
>  }
> @@ -505,7 +504,6 @@ static apr_status_t impl_pollcb_create(a
>      }
>
>      pollcb->pollset.port = apr_palloc(p, size * sizeof(port_event_t));
> -    apr_pool_cleanup_register(p, pollcb, cb_cleanup, apr_pool_cleanup_null);
>
>      return APR_SUCCESS;
>  }
> @@ -558,16 +556,25 @@ static apr_status_t impl_pollcb_poll(apr
>                                       apr_pollcb_cb_t func,
>                                       void *baton)
>  {
> -    apr_pollfd_t *pollfd;
>      apr_status_t rv;
> -    unsigned int i, nget = 1;
> +    unsigned int nget = 1;
>
>      rv = call_port_getn(pollcb->fd, pollcb->pollset.port, pollcb->nalloc,
>                          &nget, timeout);
>
>      if (nget) {
> +        unsigned int i;
> +
>          for (i = 0; i < nget; i++) {
> -            pollfd = (apr_pollfd_t *)(pollcb->pollset.port[i].portev_user);
> +            apr_pollfd_t *pollfd = (apr_pollfd_t *)(pollcb->pollset.port[i].portev_user);
> +
> +            if ((pollfd->flags & APR_POLLSET_WAKEABLE) &&
> +                pollfd->desc_type == APR_POLL_FILE &&
> +                pollfd->desc.f == pollcb->wakeup_pipe[0]) {
> +                apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe);
> +                return APR_EINTR;
> +            }
> +
>              pollfd->rtnevents = get_revent(pollcb->pollset.port[i].portev_events);
>
>              rv = func(baton, pollfd);
> @@ -586,6 +593,7 @@ static apr_pollcb_provider_t impl_cb = {
>      impl_pollcb_add,
>      impl_pollcb_remove,
>      impl_pollcb_poll,
> +    impl_pollcb_cleanup,
>      "port"
>  };

I get a compilation error (ggc on Solaris):

.../poll/unix/port.c: In function 'impl_pollcb_poll':
.../poll/unix/port.c:571:24: error: 'apr_pollfd_t' has no member named 
'flags'
              if ((pollfd->flags & APR_POLLSET_WAKEABLE) &&
                         ^

Maybe the following patch is appropriate?

--- poll/unix/port.c (revision 1755647)
+++ poll/unix/port.c (working copy)
@@ -568,7 +568,7 @@
          for (i = 0; i < nget; i++) {
              apr_pollfd_t *pollfd = (apr_pollfd_t 
*)(pollcb->pollset.port[i].portev_user);

-            if ((pollfd->flags & APR_POLLSET_WAKEABLE) &&
+            if ((pollcb->flags & APR_POLLSET_WAKEABLE) &&
                  pollfd->desc_type == APR_POLL_FILE &&
                  pollfd->desc.f == pollcb->wakeup_pipe[0]) {
                  apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe);

At least it builds and "make check" runs fine.

Regards,

Rainer

Re: svn commit: r1736521 - in /apr/apr/branches/1.6.x: ./ include/ include/arch/unix/ poll/unix/ test/

Posted by olli hauer <oh...@gmx.de>.
On 2016-08-10 14:01, Rainer Jung wrote:
> Hi Olli,
> 
> Am 10.08.2016 um 13:46 schrieb olli hauer:
>> On 2016-08-10 12:41, Rainer Jung wrote:
>>> I found some more differences between trunk and r1736521 in 1.6.x which are all due to incomplete backports of changes applied to trunk after r899905. I backported all of those because they seemed applicable for 1.6.x. At least the poll directory, testpoll.c, apr_poll.h and arch/unix/apr_arch_poll_private.h are now in sync between trunk and 1.6.x.
>>>
>>> Regards,
>>>
>>> Rainer
>>>
>>
>> Hi Rainer,
>>
>> since you are looking at the moment to apr poll, perhaps you can also take a look int Bug ID 59914?
> 
> I'm sorry: the changes I applied to poll this morning were all of very formal nature and simple to verify. I don't know enough about the code to efficiently decide on the patch that you have provided for that bug.
> 
> Regards,
> 
> Rainer
> 

Hi Rainer,

OK, thanks for the honest reply!


-- 
Regards,
olli

Re: svn commit: r1736521 - in /apr/apr/branches/1.6.x: ./ include/ include/arch/unix/ poll/unix/ test/

Posted by Rainer Jung <ra...@kippdata.de>.
Hi Olli,

Am 10.08.2016 um 13:46 schrieb olli hauer:
> On 2016-08-10 12:41, Rainer Jung wrote:
>> I found some more differences between trunk and r1736521 in 1.6.x which are all due to incomplete backports of changes applied to trunk after r899905. I backported all of those because they seemed applicable for 1.6.x. At least the poll directory, testpoll.c, apr_poll.h and arch/unix/apr_arch_poll_private.h are now in sync between trunk and 1.6.x.
>>
>> Regards,
>>
>> Rainer
>>
>
> Hi Rainer,
>
> since you are looking at the moment to apr poll, perhaps you can also take a look int Bug ID 59914?

I'm sorry: the changes I applied to poll this morning were all of very 
formal nature and simple to verify. I don't know enough about the code 
to efficiently decide on the patch that you have provided for that bug.

Regards,

Rainer

Re: svn commit: r1736521 - in /apr/apr/branches/1.6.x: ./ include/ include/arch/unix/ poll/unix/ test/

Posted by olli hauer <oh...@gmx.de>.
On 2016-08-10 12:41, Rainer Jung wrote:
> I found some more differences between trunk and r1736521 in 1.6.x which are all due to incomplete backports of changes applied to trunk after r899905. I backported all of those because they seemed applicable for 1.6.x. At least the poll directory, testpoll.c, apr_poll.h and arch/unix/apr_arch_poll_private.h are now in sync between trunk and 1.6.x.
> 
> Regards,
> 
> Rainer
> 

Hi Rainer,

since you are looking at the moment to apr poll, perhaps you can also take a look int Bug ID 59914?

-- 
Regards,

olli

Re: svn commit: r1736521 - in /apr/apr/branches/1.6.x: ./ include/ include/arch/unix/ poll/unix/ test/

Posted by Rainer Jung <ra...@kippdata.de>.
Hi Yann,

Am 10.08.2016 um 12:50 schrieb Yann Ylavic:
> Hi Rainer,
>
> On Wed, Aug 10, 2016 at 12:41 PM, Rainer Jung <ra...@kippdata.de> wrote:
>> I found some more differences between trunk and r1736521 in 1.6.x which are
>> all due to incomplete backports of changes applied to trunk after r899905. I
>> backported all of those because they seemed applicable for 1.6.x. At least
>> the poll directory, testpoll.c, apr_poll.h and
>> arch/unix/apr_arch_poll_private.h are now in sync between trunk and 1.6.x.
>
> Is 1.5.x already fine with these backports/fixes?
> APR_POLLSET_WAKEABLE is available there too...

I *think* all those changes were followups to the change that introduced 
apr_pollcb_wakeup() and apr_pollcb_method_name() to trunk. That was only 
backported to 1.6, not 1.5. The older apr_pollset_wakeup() and 
apr_pollset_method_name() shouldn't have been touched, so 1.5 should be 
still fine.

Regards,

Rainer




Re: svn commit: r1736521 - in /apr/apr/branches/1.6.x: ./ include/ include/arch/unix/ poll/unix/ test/

Posted by Yann Ylavic <yl...@gmail.com>.
Hi Rainer,

On Wed, Aug 10, 2016 at 12:41 PM, Rainer Jung <ra...@kippdata.de> wrote:
> I found some more differences between trunk and r1736521 in 1.6.x which are
> all due to incomplete backports of changes applied to trunk after r899905. I
> backported all of those because they seemed applicable for 1.6.x. At least
> the poll directory, testpoll.c, apr_poll.h and
> arch/unix/apr_arch_poll_private.h are now in sync between trunk and 1.6.x.

Is 1.5.x already fine with these backports/fixes?
APR_POLLSET_WAKEABLE is available there too...

Regards,
Yann.

Re: svn commit: r1736521 - in /apr/apr/branches/1.6.x: ./ include/ include/arch/unix/ poll/unix/ test/

Posted by Rainer Jung <ra...@kippdata.de>.
I found some more differences between trunk and r1736521 in 1.6.x which 
are all due to incomplete backports of changes applied to trunk after 
r899905. I backported all of those because they seemed applicable for 
1.6.x. At least the poll directory, testpoll.c, apr_poll.h and 
arch/unix/apr_arch_poll_private.h are now in sync between trunk and 1.6.x.

Regards,

Rainer

Am 09.08.2016 um 23:31 schrieb Rainer Jung:
> Am 25.03.2016 um 02:19 schrieb minfrin@apache.org:
>> Author: minfrin
>> Date: Fri Mar 25 01:19:34 2016
>> New Revision: 1736521
>>
>> URL: http://svn.apache.org/viewvc?rev=1736521&view=rev
>> Log:
>> Add apr_pollcb_wakeup(), with similar behavior to
>> apr_pollset_wakeup(). Add apr_pollcb_method_name(), with similar
>> behavior to apr_pollset_method_name().
>>
>
> ...
>
>> Modified: apr/apr/branches/1.6.x/poll/unix/port.c
>> URL:
>> http://svn.apache.org/viewvc/apr/apr/branches/1.6.x/poll/unix/port.c?rev=1736521&r1=1736520&r2=1736521&view=diff
>>
>> ==============================================================================
>>
>> --- apr/apr/branches/1.6.x/poll/unix/port.c (original)
>> +++ apr/apr/branches/1.6.x/poll/unix/port.c Fri Mar 25 01:19:34 2016
>> @@ -413,7 +413,7 @@ static apr_status_t impl_pollset_poll(ap
>>              if ((pollset->flags & APR_POLLSET_WAKEABLE) &&
>>                  fp.desc_type == APR_POLL_FILE &&
>>                  fp.desc.f == pollset->wakeup_pipe[0]) {
>> -                apr_pollset_drain_wakeup_pipe(pollset);
>> +                apr_poll_drain_wakeup_pipe(pollset->wakeup_pipe);
>>                  rv = APR_EINTR;
>>              }
>>              else {
>> @@ -466,9 +466,8 @@ static apr_pollset_provider_t impl = {
>>
>>  apr_pollset_provider_t *apr_pollset_provider_port = &impl;
>>
>> -static apr_status_t cb_cleanup(void *p_)
>> +static apr_status_t impl_pollcb_cleanup(apr_pollcb_t *pollcb)
>>  {
>> -    apr_pollcb_t *pollcb = (apr_pollcb_t *) p_;
>>      close(pollcb->fd);
>>      return APR_SUCCESS;
>>  }
>> @@ -505,7 +504,6 @@ static apr_status_t impl_pollcb_create(a
>>      }
>>
>>      pollcb->pollset.port = apr_palloc(p, size * sizeof(port_event_t));
>> -    apr_pool_cleanup_register(p, pollcb, cb_cleanup,
>> apr_pool_cleanup_null);
>>
>>      return APR_SUCCESS;
>>  }
>> @@ -558,16 +556,25 @@ static apr_status_t impl_pollcb_poll(apr
>>                                       apr_pollcb_cb_t func,
>>                                       void *baton)
>>  {
>> -    apr_pollfd_t *pollfd;
>>      apr_status_t rv;
>> -    unsigned int i, nget = 1;
>> +    unsigned int nget = 1;
>>
>>      rv = call_port_getn(pollcb->fd, pollcb->pollset.port,
>> pollcb->nalloc,
>>                          &nget, timeout);
>>
>>      if (nget) {
>> +        unsigned int i;
>> +
>>          for (i = 0; i < nget; i++) {
>> -            pollfd = (apr_pollfd_t
>> *)(pollcb->pollset.port[i].portev_user);
>> +            apr_pollfd_t *pollfd = (apr_pollfd_t
>> *)(pollcb->pollset.port[i].portev_user);
>> +
>> +            if ((pollfd->flags & APR_POLLSET_WAKEABLE) &&
>> +                pollfd->desc_type == APR_POLL_FILE &&
>> +                pollfd->desc.f == pollcb->wakeup_pipe[0]) {
>> +                apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe);
>> +                return APR_EINTR;
>> +            }
>> +
>>              pollfd->rtnevents =
>> get_revent(pollcb->pollset.port[i].portev_events);
>>
>>              rv = func(baton, pollfd);
>> @@ -586,6 +593,7 @@ static apr_pollcb_provider_t impl_cb = {
>>      impl_pollcb_add,
>>      impl_pollcb_remove,
>>      impl_pollcb_poll,
>> +    impl_pollcb_cleanup,
>>      "port"
>>  };
>
> I get a compilation error (ggc on Solaris):
>
> .../poll/unix/port.c: In function 'impl_pollcb_poll':
> .../poll/unix/port.c:571:24: error: 'apr_pollfd_t' has no member named
> 'flags'
>              if ((pollfd->flags & APR_POLLSET_WAKEABLE) &&
>                         ^
>
> Maybe the following patch is appropriate?
>
> --- poll/unix/port.c (revision 1755647)
> +++ poll/unix/port.c (working copy)
> @@ -568,7 +568,7 @@
>          for (i = 0; i < nget; i++) {
>              apr_pollfd_t *pollfd = (apr_pollfd_t
> *)(pollcb->pollset.port[i].portev_user);
>
> -            if ((pollfd->flags & APR_POLLSET_WAKEABLE) &&
> +            if ((pollcb->flags & APR_POLLSET_WAKEABLE) &&
>                  pollfd->desc_type == APR_POLL_FILE &&
>                  pollfd->desc.f == pollcb->wakeup_pipe[0]) {
>                  apr_poll_drain_wakeup_pipe(pollcb->wakeup_pipe);
>
> At least it builds and "make check" runs fine.
>
> Regards,
>
> Rainer