Re: Questions about apr_poll
Neil Conway wrote: pollset_wakeup shouldn't exist imo, its place is in something like the apr_events_* api that was proposed and never adopted at one point, but it was added anyways because of a use case needed by tomcat nc (?) I agree that a higher-level events API would be useful, but in the absence of that, pollcb_wakeup() would be pretty useful: wakeup() + a secondary event queue is a reasonable substitute for a general-purpose event notification system. IMHO a lot of people will basically need to roll this functionality by hand in the absence of a builtin pollcb_wakeup(). If a patch for pollcb_wakeup() would be considered or accepted, I can look into writing one. +1 for a pollcb_wakeup(). Sure, an apr_events API would be an excellent thing to have, but we don't have it right now, and people have problems they need solved. Regards, Graham --
Re: [PATCH] doxygen fixes
Neil Conway wrote: Attached is a straightforward patch against svn trunk that fixes various doxygen usage warnings and minor errors. Committed to trunk, and backported to v1.4 and v1.3. Regards, Graham --
RE: Windows drive letter check fails on lower case cwd
-Original Message- From: Bert Huijben [mailto:b...@qqmail.nl] Sent: maandag 9 november 2009 10:05 To: 'William A. Rowe, Jr.' Cc: 'Philip Martin'; 'Bert Huijben'; dev@apr.apache.org; d...@subversion.tigris.org Subject: RE: Windows drive letter check fails on lower case cwd -Original Message- From: William A. Rowe, Jr. [mailto:wr...@rowe-clan.net] Sent: maandag 9 november 2009 4:25 To: Bert Huijben Cc: 'Philip Martin'; 'Bert Huijben'; dev@apr.apache.org; d...@subversion.tigris.org Subject: Re: Windows drive letter check fails on lower case cwd Bert Huijben wrote: The drive letters don't have locales; the rest of the paths have. There are only 26 driveletters with the US-ASCII characters A-Z. (Internally always represented by the upper case letters, but the current path can use a lower case path, as that is only managed in userspace). That's an odd statement; all paths are Unicode ;-) We don't strcasecmp paths. s/path/drive in its path/ for that last path.. thanks :) Paths are (of course) unicode, but use a some culture sensitive compare to handle the case insensitivity. But there is no documented way to find which locale it uses for each (part of a) drive. (That information is stored in the system portion of NTFS at format time and can vary over directories via junctions, etc.) Thanks for looking into this. After some delay I created a new patch with the suggested changes and a testcase for inclusion in the apr testsuite. (Note that the test can falsely succeed if your test environment is on C:) I tested this patch on the 1.4.x branch and 1.3.x, but created the patch based on trunk. (The visual studio projects don't include the expat build, but it is required for a successful compilation). As noted earlier, this issue will be experienced by far more users once Subversion 1.7 is released, so it would be nice if it is backported to the relevant releases. Bert lowercase_drive_merge.patch Description: Binary data
Re: Questions about apr_poll
Neil Conway wrote: Attached is a patch against trunk that updates the documentation for apr_poll to describe this behavior. I also fixed a few minor doxygen issues along the way. Committed to trunk and v1.4. Regards, Graham --
Re: Questions about apr_poll
Another minor point on a related subject: the documentation for pollcb_create() states that the size parameter controls The maximum number of descriptors that a single _poll can return. This isn't true for the poll(2) pollcb method, however: _add() returns APR_ENOMEM if the number of active descriptors exceeds the initial size. Neil On Tue, Jan 5, 2010 at 12:16 AM, Neil Conway n...@cs.berkeley.edu wrote: (1) The documentation for apr_pollset_create() states that the size parameter controls The maximum number of descriptors that this pollset can hold. However, ancient history like http://www.mail-archive.com/dev@apr.apache.org/msg12876.html suggests that size now just controls the # of events that can be signalled by a single call to apr_pollset_poll(). From glancing at the code, it seems that the select provider has a fixed limit on the pollset size, but kqueue, epoll and event port do not, for example. Is that accurate? Perhaps the apr_pollset documentation can be updated accordingly. (2) Is there a reason why apr_pollset_wakeup() was added, but there's no analogous apr_pollcb_wakeup()? Speaking of which, it might be good to document the high-level differences between apr_pollset and apr_pollcb -- without doing some digging, it is not at all clear how they differ, or why one might prefer one over the other. Thanks in advance for any advice, Neil
[PATCH] bug in pollset_wakeup() + nocopy
apr_pollset_wakeup() is buggy when used with APR_POLLSET_NOCOPY, because create_wakeup_pipe() passes a stack-allocated apr_pollfd_t to apr_pollset_add(). This is unsafe if the user specified APR_POLLSET_NOCOPY when creating the pollset. The attached patch fixes this by adding an apr_pollfd_t for the wakeup pipe to apr_pollset_t, so that it has a sufficiently-long-lived lifetime. Neil apr_pollset_wakeup_nocopy-1.patch Description: Binary data
Re: [PATCH] bug in pollset_wakeup() + nocopy
Attached is a slightly revised version of this patch. Changes: * Initialize the apr_pool_t field of the apr_pollfd_t we use for the wakeup pipe. Not clear what this field is actually used for (candidate for removal in APR2?), but we may as well be tidy. * Fix a minor bug in one of the versions of close_wakeup_pipe(): initialize both rv0 and rv1, to avoid potentially reading an uninitialized value. Neil On Wed, Jan 6, 2010 at 5:47 PM, Neil Conway n...@cs.berkeley.edu wrote: apr_pollset_wakeup() is buggy when used with APR_POLLSET_NOCOPY, because create_wakeup_pipe() passes a stack-allocated apr_pollfd_t to apr_pollset_add(). This is unsafe if the user specified APR_POLLSET_NOCOPY when creating the pollset. The attached patch fixes this by adding an apr_pollfd_t for the wakeup pipe to apr_pollset_t, so that it has a sufficiently-long-lived lifetime. Neil apr_pollset_wakeup_nocopy-2.patch Description: Binary data
