Re: svn commit: r920017 - in /apr/apr/branches/1.4.x: ./ file_io/unix/open.c include/apr_file_io.h

[Joe Orton <jo...@redhat.com>]

On Mon, Mar 08, 2010 at 03:53:42PM -0500, Jeff Trawick wrote:
> On Mon, Mar 8, 2010 at 9:59 AM, Graham Leggett <mi...@sharp.fm> wrote:
...
> > #if APR_HAS_LARGE_FILES && defined(_LARGEFILE64_SOURCE)
> >    oflags |= O_LARGEFILE;
> > #elif defined(O_LARGEFILE)
> >    if (flag & APR_FOPEN_LARGEFILE) {
> >        oflags |= O_LARGEFILE;
> >    }
> > #endif
> 
> This might be more interesting., though perhaps when the
> configure-time logic to turn on large file support is out of sync with
> this code?  I'm not sure...

APR_FOPEN_LARGEFILE is intended to be (and is documented as) "advisory" 
rather than mandatory.

Exposing a NONBLOCK flag without attaching *any* semantics to it w.r.t. 
subsequent read/write calls seems completely wrong.  If you want 
platform-specific behaviour and don't care how badly it interacts with 
other apr_file_* interfaces then use native open() and apr_os_file_put() 
to get the APR wrapper.

Regards, Joe

Re: svn commit: r920017 - in /apr/apr/branches/1.4.x: ./ file_io/unix/open.c include/apr_file_io.h

["William A. Rowe Jr." <wr...@rowe-clan.net>]

On 3/13/2010 5:44 PM, Bojan Smojver wrote:
> On Fri, 2010-03-12 at 13:30 +0000, Joe Orton wrote:
>> I'd expect to see some description of exactly what the new APR_FOPEN_*
>> flag changes w.r.t. method calls; does it affect just read/write, what
>> about flush, sync, etc? Can I presume POSIX semantics w.r.t.
>> O_NONBLOCK in open()?  The questions are kind of endless if the
>> documentation isn't there.
> 
> One way to figure this out for sure would be to enhance the test suite
> so it exercises relevant tests using non blocking semantics, I guess.

+1

Re: svn commit: r920017 - in /apr/apr/branches/1.4.x: ./ file_io/unix/open.c include/apr_file_io.h

[Bojan Smojver <bo...@rexursive.com>]

On Fri, 2010-03-12 at 13:30 +0000, Joe Orton wrote:
> I'd expect to see some description of exactly what the new APR_FOPEN_*
> flag changes w.r.t. method calls; does it affect just read/write, what
> about flush, sync, etc? Can I presume POSIX semantics w.r.t.
> O_NONBLOCK in open()?  The questions are kind of endless if the
> documentation isn't there.

One way to figure this out for sure would be to enhance the test suite
so it exercises relevant tests using non blocking semantics, I guess.

-- 
Bojan

Re: svn commit: r920017 - in /apr/apr/branches/1.4.x: ./ file_io/unix/open.c include/apr_file_io.h

[Bojan Smojver <bo...@rexursive.com>]

On Sat, 2010-03-13 at 17:54 +0200, Graham Leggett wrote:
> Based on what you've written above, the #else ENOTIMPL seems to be the
> way to go. If it isn't supported, you get explicit confirmation of the
> fact 

Probably the safe thing to do. Probably won't get many hits, but
nevertheless.

-- 
Bojan

Re: svn commit: r920017 - in /apr/apr/branches/1.4.x: ./ file_io/unix/open.c include/apr_file_io.h

["William A. Rowe Jr." <wr...@rowe-clan.net>]

On 3/13/2010 9:54 AM, Graham Leggett wrote:
> 
> To zoom out a bit to put this into context, right now APR has no support
> for non blocking behaviour at all, and this is the problem I would like
> to fix. Using apr_os_file_put(), while functional, isn't portable, and
> as a result a user of the API is forced to have "if windows do this, if
> unix do that". This is APR's job, not the job of the application using APR.

You are aboslutely right; whatever goes into the apr_file api for nonblocking
file behavior must be portable, work consistently and produce identical results
on all platforms.  What I think Joe (and I) have been suggesting is that this
API does not look thought out from the perspective you have identified above.

> My understanding is that these are well understood under unix, are there
> unix platforms out there that *don't* support non blocking behaviour?

It is not surprising how distinct the BSD, Linux and Solaris "standard"
behaviors are; why else does virtually every die-hard APR developer rely
on a copy of Stevens "Unix Network Programming" on the shelf?

> In the Windows case we can certainly return ENOTIMPL, and the same for
> Netware (the Netware people can change that to be otherwise if it isn't
> true).

Why would we?  I'm under the impression that file I/O completion had
existed on WindowsNT for about 2 decades.  It need not block.  Now define
nonblocking behavior, and we are all set.  I suspect that the entire unix
file read/write set of API's, including write_complete etc, need to be
reviewed in light of the introduction of this flag.

I guess I'm perceiving this as throwing a unix feature at APR to see if
it sticks, and I'm not fond of that approach to such junk expansion of APR.

Re: svn commit: r920017 - in /apr/apr/branches/1.4.x: ./ file_io/unix/open.c include/apr_file_io.h

[Graham Leggett <mi...@sharp.fm>]

On 12 Mar 2010, at 3:30 PM, Joe Orton wrote:

> Well, right, this the point I was making - it should be no different  
> at
> run-time; i.e. you can achieve exactly what you want already without
> adding a new interface which uses up one of the remaining bits in the
> APR_FOPEN_* bitmask, must be maintained in perpuity, won't work on
> platform X and hence confuses users, etc etc.
>
> But specifically, if I'm writing an app which depends on being able to
> do non-blocking IO to a fifo, I *need* the API guarantee that  
> O_NONBLOCK
> works.  You can get that with open()+apr_os_file_put(), but you can't
> get that with the API added to APR, which has no such guarantee.  So
> from that perspective, adding _NONBLOCK to APR is little use.
>
> To be clear; I don't see a problem with adding new flags so long as  
> they
> come with documented API constraints/guarantees.  It's the idea of
> adding a seemingly deliberately undocumented platform-specific flag
> which seems wrong.  Does that make more sense?

To zoom out a bit to put this into context, right now APR has no  
support for non blocking behaviour at all, and this is the problem I  
would like to fix. Using apr_os_file_put(), while functional, isn't  
portable, and as a result a user of the API is forced to have "if  
windows do this, if unix do that". This is APR's job, not the job of  
the application using APR.

Based on what you've written above, the #else ENOTIMPL seems to be the  
way to go. If it isn't supported, you get explicit confirmation of the  
fact, and the caller can then make a decision as to what to do at that  
point (leave out the flag, punt, whatever).

>> Are there specific semantics you are referring to, or are you just
>> assuming they are missing? In the case of read and write,
>> APR_STATUS_IS_EAGAIN already gives you the semantics you want. Or
>> are you referring to something else?
>
> I'd expect to see some description of exactly what the new APR_FOPEN_*
> flag changes w.r.t. method calls; does it affect just read/write, what
> about flush, sync, etc? Can I presume POSIX semantics w.r.t.  
> O_NONBLOCK
> in open()?  The questions are kind of endless if the documentation  
> isn't
> there.

My understanding is that these are well understood under unix, are  
there unix platforms out there that *don't* support non blocking  
behaviour?

What I'm trying to ascertain is whether we are trying to solve a  
problem that isn't there. I'm not convinced we need the ifdef  
O_NONBLOCK at all in the unix case.

In the Windows case we can certainly return ENOTIMPL, and the same for  
Netware (the Netware people can change that to be otherwise if it  
isn't true).

Regards,
Graham
--

Re: svn commit: r920017 - in /apr/apr/branches/1.4.x: ./ file_io/unix/open.c include/apr_file_io.h

[Joe Orton <jo...@redhat.com>]

On Tue, Mar 09, 2010 at 12:26:08PM +0200, Graham Leggett wrote:
> On 08 Mar 2010, at 11:26 PM, Joe Orton wrote:
> 
> >APR_FOPEN_LARGEFILE is intended to be (and is documented as)
> >"advisory"
> >rather than mandatory.
> >
> >Exposing a NONBLOCK flag without attaching *any* semantics to it
> >w.r.t.
> >subsequent read/write calls seems completely wrong.  If you want
> >platform-specific behaviour and don't care how badly it interacts with
> >other apr_file_* interfaces then use native open() and
> >apr_os_file_put()
> >to get the APR wrapper.
> 
> This makes no sense at all.
> 
> If I open a file using native open() setting flags appropriately,
> and then use apr_os_file_put(), followed by apr_file_read and
> apr_file_write, how is that in any way different to using
> apr_file_open(), setting flags appropriately, and then follow with
> apr_file_read and apr_file_write?

Well, right, this the point I was making - it should be no different at 
run-time; i.e. you can achieve exactly what you want already without 
adding a new interface which uses up one of the remaining bits in the 
APR_FOPEN_* bitmask, must be maintained in perpuity, won't work on 
platform X and hence confuses users, etc etc.

But specifically, if I'm writing an app which depends on being able to 
do non-blocking IO to a fifo, I *need* the API guarantee that O_NONBLOCK 
works.  You can get that with open()+apr_os_file_put(), but you can't 
get that with the API added to APR, which has no such guarantee.  So 
from that perspective, adding _NONBLOCK to APR is little use.

To be clear; I don't see a problem with adding new flags so long as they 
come with documented API constraints/guarantees.  It's the idea of 
adding a seemingly deliberately undocumented platform-specific flag 
which seems wrong.  Does that make more sense?

> Are there specific semantics you are referring to, or are you just
> assuming they are missing? In the case of read and write,
> APR_STATUS_IS_EAGAIN already gives you the semantics you want. Or
> are you referring to something else?

I'd expect to see some description of exactly what the new APR_FOPEN_* 
flag changes w.r.t. method calls; does it affect just read/write, what 
about flush, sync, etc? Can I presume POSIX semantics w.r.t. O_NONBLOCK 
in open()?  The questions are kind of endless if the documentation isn't 
there.

Regards, Joe

Re: svn commit: r920017 - in /apr/apr/branches/1.4.x: ./ file_io/unix/open.c include/apr_file_io.h

[Graham Leggett <mi...@sharp.fm>]

On 08 Mar 2010, at 11:26 PM, Joe Orton wrote:

> APR_FOPEN_LARGEFILE is intended to be (and is documented as)  
> "advisory"
> rather than mandatory.
>
> Exposing a NONBLOCK flag without attaching *any* semantics to it  
> w.r.t.
> subsequent read/write calls seems completely wrong.  If you want
> platform-specific behaviour and don't care how badly it interacts with
> other apr_file_* interfaces then use native open() and  
> apr_os_file_put()
> to get the APR wrapper.

This makes no sense at all.

If I open a file using native open() setting flags appropriately, and  
then use apr_os_file_put(), followed by apr_file_read and  
apr_file_write, how is that in any way different to using  
apr_file_open(), setting flags appropriately, and then follow with  
apr_file_read and apr_file_write?

Are there specific semantics you are referring to, or are you just  
assuming they are missing? In the case of read and write,  
APR_STATUS_IS_EAGAIN already gives you the semantics you want. Or are  
you referring to something else?

Regards,
Graham
--