httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Kraemer <mar...@hyperreal.org>
Subject [APR] Comments to proposed function interface
Date Fri, 22 Jan 1999 21:15:51 GMT
Okay, here are some thoughts I just had to write down when I
first read the apr-function.txt draft.
Food for thought - please criticize me / suggest improvements. 

    Martin
---------------
*** Types ***

I was somewhat astonished to see 64 bit sizes hardwired into the
spec. (apr_write() apparently uses a 64 bit size, even on 32 bit
unixes.) OTOH, the function apr_format_time() uses two 32 bit
arguments to specify the size of an in-memory object (even on a
64 bit architecture).

!!PLEASE!! Don't hardwire either 32 bit or 64 bit into the
interface! That's what size_t was invented for. The apr
abstraction should be especially cautious with topics like this. 

All parameters which specify the the size of an in-memory-object
should be `APRSize' with an appropriate typedef somewhere in the
platform specific parts.

Likewise, a file offset should have a type `APRFileOffset'
(instead of APRInt64), and a time value should be `APRTimeValue'
(instead of APRUInt32). Where necessary, these quantities will
have to be converted/expanded/truncated to make them fit the
OS's interfaces (like in apr_listen() <-> listen()).
That's why we create the APR abstraction in the first place.

The port number for network connections should also be
`APRPortNo', not APRUInt16.

APRstatus 

While this may be a VMS specialty, the choice of status fields
does not look very well thought-out to me. While it is good to
have a severity level (I remember "I"nformatory, "W"arning,
"E"rror and "U"nrecoverable error flags back from my old OS/360
days), and a separate "problem" flag, plus an error message
number value, I think that putting the severity into the LOWEST
three bits (and combining them with the "problem" bit) isn't
such a terribly good idea. What strikes me especially is the
fact that the severity level isn't ascending (or I must have
misunderstood the meaning of "Warning" vs. "Unqualified success"
or "Informational". At IBM's, an information is less severe than
a warning, is it?).

Then there's arbitrary constants like 0xFFF8 denoting "errors
from any origin". Why not use the value 0 here? 

What I propose is to leave the AND'ing, OR'ing and SHIFT'ing all
to the compiler (we're not living in assembler days anymore!)
and use the vehicle of structures (and unions?) to achieve the
same result, albeit with an easier interface:

typedef struct {
  enum {
    svty_OK,
    svty_WARNING,	/* add more severity levels? */
    svty_ERROR,
    svty_FATAL
  } severity;

  enum {
    orig_ANY = 0,
    orig_FILESYS,
    orig_SHELL,
    orig_INTERNAL
    /*...*/
  } origin;

  enum {
    rslt_SUCCESS = 0,
    rslt_NO_MEMORY,
    rslt_NO_PERMISSION,
    rslt_NO_MORE_SOCKETS /* invent more as you like */
    /*....*/
  } result;

} APRstatus;

If we're really worried about space (which I doubt in times of
MBytes and GBytes of installed ram), OR if it is important to
have "error constants", then one could still fall back to bit
fields and create a union of an unsigned long (and it wouldn't
matter if a long is 4 or 8 bytes long) with the bit masks:  

typedef union {
  unsigned long val;     /* the bits combined into a long */
  struct {
   unsigned severity:3;  /* svty_OK ... svty_FATAL*/
   unsigned origin:13;
   short    result:16;   /* rslt_SUCCESS... */
  } bits;
} APRstatus;

enum {
  stat_SUCCESS = 0x0000,
  stat_FILE_SYSTEM_FULL = 0xCAFE...
};

*** Passing Buffers ***

Some functions in the spec require a buffers of the correct size
to be passed down which is then filled in by the function. I
think that this can be avoided in many cases (especially in
cases where the size isn't known beforehand, like in
apr_netaddrtostring() or apr_format_time()). Since we pass a
pool pointer to most of these functions, it only seems natural
to allocate a string in the function and return that instead of
having to check everywhere whether the passed buffer is big
enough. 


--- Functions for File I/O ---

apr_open(): 
  A flag for binary files is needed for DOS / EBCDIC based systems.

apr_opendir()
  This function is too *nix centric. A good abstraction could be
created with a callback interface where the caller supplies a
function which is called for each file in a directory. Its
return value would determine if more scanning is desired or if the
caller found the corresponding file already, or if the entry
should be added to an automatically created (and sorted?) array.
While the BSD4.3 scandir() function could serve as an example
interface, we could define a different callback model as well.
Such a function would easily wrap the unix
opendir/readdir/closedir calls, the DOS (and even CP/M) specific
findfirst/findnext functionality and other OS models.

--- Network I/O ---
IPV6 requirements have not been addressed. Are all network
interfaces "ready for IPV6"?

apr_enumeratehostent()
  Like in the directory scan case, the natural solution to an
unknown number of result entries is not building the loop at
each calling location, but to use a callback and integrate the
loop into the function. The callback function can then do
whatever it wants with the individual entries.
Alternatively, the apr_enumeratehostent() function could return
the number of entries and a complete array of entries which
could simply be indexed.
The proposed interface (start with an index value of zero and 
continue until the "next value" is returned as 0) is to
deadloop-prone. It seems more natural to terminate the loop with
a value that can not be used as a valid index (-1).


--- Locks ---
apr_lock()
  There is no non-blocking way to test a lock.

--- Processes ---
Is there a way to differentiate between detached vs.
"foreground" processes?

-- 
Martin Kraemer                                         <Martin@apache.org>

Mime
View raw message