httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brian Havard" <bri...@kheldar.apana.org.au>
Subject Re: APR file i/o questions
Date Fri, 16 Jun 2000 09:05:08 GMT
On Wed, 14 Jun 2000 17:25:05 -0400, Jeff Trawick wrote:

>Should the read-type functions return APR_EOF at end-of-file on all
>platforms?
>
>  (Plenty of folks seem to agree: Ryan, Jeff, Brian, James)

That's how I see it yes. The way it is, I ask for some data to be read, it
reads nothing yet still returns APR_SUCCESS. To me that's nonsense.

I'm not decided about the case where the whole buffer isn't filled though
I'm leaning towards leaving that the way it is (returning success).



>Should ap_ungetc() work on all files, buffered or not?
>
>  (Jeff thinks so.)

Yes, though I'm not sure it's needed in its current form. What we actually
want is the ability to back up 1 char, IE seek -1. We don't need to be able
to specify what the ungot character is.



>Should ap_fgets() on Unix have to check for '\r' *and* '\n' (i.e.,
>think about '\r' at all)?
>
>  (Jeff thinks not.  fgets() has managed without it forever)

Probably not. I did that for the OS/2 & Win32 code as they'll usually get
CRLF files. Another point though is should the trailing \n be left in the
returned string? I know thats what the standard fgets does but I've always
found that annoying. Anyone else think that?



>What about locking within the buffered I/O support?
>
>  (Jeff thinks that by default there should be no locking, but that a
>  flag on ap_open() (APR_MT) can turn it on.  This flag would be
>  ignored (not failed) if !APR_HAS_THREADS.  Even in a multithreaded
>  app it seems uncommon for more than one thread to access the same
>  file at once, so this is a reasonable default.

Yup, I agree fully.



>What about Jeff's patch to Unix readwrite.c he posted earlier?
>
>  (Jeff is 70% sure he should change it to call ap_read() from
>  ap_fgets() but he will wait until there is a chance for some answers
>  to these questions before playing more in the code.  Hopefully the
>  next time he is coding there he can handle the resolution of more of
>  the questions above.)

I've got it working with just this (yes, I have a linux box too):


Index: file_io/unix/open.c
===================================================================
RCS file: /home/cvs/apache-2.0/src/lib/apr/file_io/unix/open.c,v
retrieving revision 1.54
diff -u -w -r1.54 open.c
--- file_io/unix/open.c	2000/06/15 17:39:16	1.54
+++ file_io/unix/open.c	2000/06/16 08:40:59
@@ -112,6 +112,10 @@
 
     (*new)->buffered = (flag & APR_BUFFERED) > 0;
 
+    if ((*new)->buffered) {
+        (*new)->buffer = ap_palloc(cont, APR_FILE_BUFSIZE);
+    }
+
     if (flag & APR_CREATE) {
         oflags |= O_CREAT; 
 	if (flag & APR_EXCL) {
Index: file_io/unix/readwrite.c
===================================================================
RCS file: /home/cvs/apache-2.0/src/lib/apr/file_io/unix/readwrite.c,v
retrieving revision 1.54
diff -u -w -r1.54 readwrite.c
--- file_io/unix/readwrite.c	2000/06/14 02:58:34	1.54
+++ file_io/unix/readwrite.c	2000/06/16 08:41:01
@@ -301,6 +301,7 @@
 
 ap_status_t ap_getc(char *ch, ap_file_t *thefile)
 {
+    ap_status_t status;
     ssize_t rv;
     
     if (thefile->ungetchar != -1) {
@@ -308,13 +309,15 @@
         thefile->ungetchar = -1;
         return APR_SUCCESS;
     }
-    rv = read(thefile->filedes, ch, 1); 
+
+    rv = 1;
+    status = ap_read(thefile, ch, &rv);
     if (rv == 0) {
         thefile->eof_hit = TRUE;
         return APR_EOF;
     }
     else if (rv != 1) {
-        return errno;
+        return status;
     }
     return APR_SUCCESS; 
 }
@@ -358,6 +361,7 @@
 {
     ssize_t rv;
     int i, used_unget = FALSE, beg_idx;
+    ap_status_t status;
 
     if (len <= 1) {  /* as per fgets() */
         return APR_SUCCESS;
@@ -378,7 +382,8 @@
     }
     
     for (i = beg_idx; i < len; i++) {
-        rv = read(thefile->filedes, &str[i], 1); 
+        rv = 1;
+        status = ap_read(thefile, str+i, &rv);
         if (rv == 0) {
             thefile->eof_hit = TRUE;
 	    if (used_unget) {
@@ -388,7 +393,7 @@
             return APR_EOF;
         }
         else if (rv != 1) {
-            return errno;
+            return status;
         }
         if (str[i] == '\n' || str[i] == '\r') {
             break;

-- 
 ______________________________________________________________________________
 |  Brian Havard                 |  "He is not the messiah!                   |
 |  brianh@kheldar.apana.org.au  |  He's a very naughty boy!" - Life of Brian |
 ------------------------------------------------------------------------------


Mime
View raw message