You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Paul Sutton <pa...@ukweb.com> on 1997/06/20 20:54:20 UTC

OS abstraction

I think it would be nice to remove, as far as possible, the #ifdef __EMX__
and #ifdef WIN32 sections from the core Apache core. 

Looking at the code there are a lot of "little" ifdef sections to cope
with things like allowing drive: at the start of filenames on EMX and
WIN32 but not Unix. Quite often the ifdef (and the code) is repeated -
because under Unix the repeated code is trivial. 

As an example, here are some samples of the sorts of abstraction that can
be done to make the core code neater and less repeatative. I've assumed
there will be an OS-specific header file for each of Unix, EMX and Win32
(the appropriate one can be included from conf.h for now, until there is a
proper way of selecting an OS). For each item that is abstracted, there is
a macro defined in the appropriate OS header file. 

For large ifdef sections in the code, it would be nicer to have defines
for the capabilities of the OS. For example, Unix could define
HAVE_SYMLINKS and the ifdef around check_symlinks could be a #ifdef
HAVE_SYMLINKS instead of checking for EMX and Win32 specifically. That is
we move the OS specific code completely out of the core into a well define
place (the OS header file). It will make it easier to manage OS specific
issues. 

So, for this example I've abstracted four typical OS-specific items: file
permissions, a function call (pipe), and a filesystem difference and a
case where the filesystem difference impacts on URI processing. The three
OS header files look like this: 

os-unix.h

 #define AP_FILE_CREATE_PERMS \
              S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH
 #define ap_pipe(fds) pipe(fds)
 #define ap_is_path_absolute(file) (file && file[0] == '/' ? 1 : 0)
 #define ap_guess_if_path_is_uri(x) (strchr(x, ':')!= NULL)

os-emx.h

 #define AP_FILE_CREATE_PERMS _S_IREAD | _S_IWRITE
 #define ap_pipe(fds) pipe(fds)
 #define ap_is_path_absolute(file) \
   ((file && ((file[0] == '/') || (file[0] != '\0' && file[1] == ':')))?1:0)
 #define ap_guess_if_path_is_uri(x) (strchr(x+2, ':') != NULL)

os-win32.h

 #define AP_FILE_CREATE_PERMS _S_IREAD | _S_IWRITE
 #define ap_pipe(fds) _pipe(fds, 512, O_TEXT | O_NOINHERIT)
 #define ap_is_path_absolute(file) \
   ((file && ((file[0] == '/') || (file[0] != '\0' && file[1] == ':')))?1:0)
 #define ap_guess_if_path_is_uri(x) (strchr(x+2, ':') != NULL)

Finally I'll enclose a patch which shows how these abstractions make the
code a bit neater (in my opinion anyway). Even if this particular
implementation isn't used I'd like to get the ball rolling on abstracting
OS specific things out of Apache (e.g. using Like Doug's ApacheIO stuff). 

//pcs

Index: alloc.c
===================================================================
RCS file: /export/home/cvs/apache/src/alloc.c,v
retrieving revision 1.32
diff -c -r1.32 alloc.c
*** alloc.c	1997/06/16 18:42:47	1.32
--- alloc.c	1997/06/20 18:21:54
***************
*** 870,888 ****
    int baseFlag, desc;
    int modeFlags = 0;
  
- #ifdef WIN32
-   modeFlags = _S_IREAD | _S_IWRITE;
- #else
-   modeFlags = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
- #endif
- 
    block_alarms();
  
    if (*mode == 'a') {
      /* Work around faulty implementations of fopen */
      baseFlag = (*(mode+1) == '+') ? O_RDWR : O_WRONLY;
      desc = open(name, baseFlag | O_APPEND | O_CREAT,
! 		modeFlags);
      if (desc >= 0) {
        fd = fdopen(desc, mode);
      }
--- 870,882 ----
    int baseFlag, desc;
    int modeFlags = 0;
  
    block_alarms();
  
    if (*mode == 'a') {
      /* Work around faulty implementations of fopen */
      baseFlag = (*(mode+1) == '+') ? O_RDWR : O_WRONLY;
      desc = open(name, baseFlag | O_APPEND | O_CREAT,
! 		AP_FILE_CREATE_PERMS);
      if (desc >= 0) {
        fd = fdopen(desc, mode);
      }
***************
*** 1017,1028 ****
    a->subprocesses = new;
  }
  
- #ifdef WIN32
- #define enc_pipe(fds) _pipe(fds, 512, O_TEXT | O_NOINHERIT)
- #else
- #define enc_pipe(fds) pipe(fds)
- #endif /* WIN32 */
- 
  int spawn_child_err (pool *p, int (*func)(void *), void *data,
  		     enum kill_conditions kill_how,
  		     FILE **pipe_in, FILE **pipe_out, FILE **pipe_err)
--- 1011,1016 ----
***************
*** 1035,1041 ****
  
    block_alarms();
    
!   if (pipe_in && enc_pipe (in_fds) < 0)
    {
        save_errno = errno;
        unblock_alarms();
--- 1023,1029 ----
  
    block_alarms();
    
!   if (pipe_in && ap_pipe (in_fds) < 0)
    {
        save_errno = errno;
        unblock_alarms();
***************
*** 1043,1049 ****
        return 0;
    }
    
!   if (pipe_out && enc_pipe (out_fds) < 0) {
      save_errno = errno;
      if (pipe_in) {
        close (in_fds[0]); close (in_fds[1]);
--- 1031,1037 ----
        return 0;
    }
    
!   if (pipe_out && ap_pipe (out_fds) < 0) {
      save_errno = errno;
      if (pipe_in) {
        close (in_fds[0]); close (in_fds[1]);
***************
*** 1053,1059 ****
      return 0;
    }
  
!   if (pipe_err && enc_pipe (err_fds) < 0) {
      save_errno = errno;
      if (pipe_in) {
        close (in_fds[0]); close (in_fds[1]);
--- 1041,1047 ----
      return 0;
    }
  
!   if (pipe_err && ap_pipe (err_fds) < 0) {
      save_errno = errno;
      if (pipe_in) {
        close (in_fds[0]); close (in_fds[1]);
***************
*** 1196,1232 ****
    
    if (pipe_out) {
      close (out_fds[1]);
! #ifdef __EMX__
!     /* Need binary mode set for OS/2. */
!     *pipe_out = fdopen (out_fds[0], "rb");
! #else
!     *pipe_out = fdopen (out_fds[0], "r");
! #endif  
!   
      if (*pipe_out) note_cleanups_for_file (p, *pipe_out);
    }
  
    if (pipe_in) {
      close (in_fds[0]);
! #ifdef __EMX__
!     /* Need binary mode set for OS/2 */
!     *pipe_in = fdopen (in_fds[1], "wb");
! #else
!     *pipe_in = fdopen (in_fds[1], "w");
! #endif
!     
      if (*pipe_in) note_cleanups_for_file (p, *pipe_in);
    }
  
    if (pipe_err) {
      close (err_fds[1]);
! #ifdef __EMX__
!     /* Need binary mode set for OS/2. */
!     *pipe_err = fdopen (err_fds[0], "rb");
! #else
!     *pipe_err = fdopen (err_fds[0], "r");
! #endif
!   
      if (*pipe_err) note_cleanups_for_file (p, *pipe_err);
    }
  #endif /* WIN32 */
--- 1184,1202 ----
    
    if (pipe_out) {
      close (out_fds[1]);
!     *pipe_out = ap_fdopenb (out_fds[0], "r");
      if (*pipe_out) note_cleanups_for_file (p, *pipe_out);
    }
  
    if (pipe_in) {
      close (in_fds[0]);
!     *pipe_in = ap_fdopenb (in_fds[1], "w");
      if (*pipe_in) note_cleanups_for_file (p, *pipe_in);
    }
  
    if (pipe_err) {
      close (err_fds[1]);
!     *pipe_err = ap_fdopenb (err_fds[0], "r");
      if (*pipe_err) note_cleanups_for_file (p, *pipe_err);
    }
  #endif /* WIN32 */
Index: conf.h
===================================================================
RCS file: /export/home/cvs/apache/src/conf.h,v
retrieving revision 1.101
diff -c -r1.101 conf.h
*** conf.h	1997/06/16 20:04:51	1.101
--- conf.h	1997/06/20 18:22:02
***************
*** 55,60 ****
--- 55,62 ----
   * See README for a listing of what they mean
   */
  
+ #include "nt/os-unix.h"
+ 
  #if !defined(QNX) && !defined(MPE) && !defined(WIN32)
  #include <sys/param.h>
  #endif
Index: http_request.c
===================================================================
RCS file: /export/home/cvs/apache/src/http_request.c,v
retrieving revision 1.51
diff -c -r1.51 http_request.c
*** http_request.c	1997/06/15 19:22:27	1.51
--- http_request.c	1997/06/20 18:22:51
***************
*** 259,270 ****
       * for the moment, that's not worth the trouble.
       */
  
! #if defined(__EMX__) || defined(WIN32)
!     /* Add OS/2 drive name support */
!     if ((test_filename[0] != '/') && (test_filename[1] != ':'))
! #else
!     if (test_filename[0] != '/')
! #endif
      {
  /* fake filenames only match Directory sections */
          void *this_conf, *entry_config;
--- 259,265 ----
       * for the moment, that's not worth the trouble.
       */
  
!     if (!ab_is_path_absolute(file))
      {
  /* fake filenames only match Directory sections */
          void *this_conf, *entry_config;
Index: mod_alias.c
===================================================================
RCS file: /export/home/cvs/apache/src/mod_alias.c,v
retrieving revision 1.16
diff -c -r1.16 mod_alias.c
*** mod_alias.c	1997/06/15 19:22:28	1.16
--- mod_alias.c	1997/06/20 18:22:59
***************
*** 263,274 ****
      char *ret;
      int status;
  
! #if defined(__EMX__) || defined(WIN32)
!     /* Add support for OS/2 drive names */
!     if ((r->uri[0] != '/' && r->uri[0] != '\0') && r->uri[1] != ':')
! #else    
!     if (r->uri[0] != '/' && r->uri[0] != '\0') 
! #endif    
          return DECLINED;
  
      if ((ret = try_alias_list (r, serverconf->redirects, 1, &status)) != NULL) {
--- 263,269 ----
      char *ret;
      int status;
  
!     if (!ap_is_path_absolute(r->uri) && r->uri[0] != '\0') 
          return DECLINED;
  
      if ((ret = try_alias_list (r, serverconf->redirects, 1, &status)) != NULL) {
Index: mod_userdir.c
===================================================================
RCS file: /export/home/cvs/apache/src/mod_userdir.c,v
retrieving revision 1.15
diff -c -r1.15 mod_userdir.c
*** mod_userdir.c	1997/06/15 19:22:32	1.15
--- mod_userdir.c	1997/06/20 18:23:10
***************
*** 142,166 ****
        if (strchr(userdir, '*'))
  	x = getword(r->pool, &userdir, '*');
  
! #if defined(__EMX__) || defined(WIN32)
!       /* Add support for OS/2 drive letters */
!       if ((userdir[0] == '/') || (userdir[1] == ':') || (userdir[0] == '\0')) {
! #else
!       if ((userdir[0] == '/') || (userdir[0] == '\0')) {
! #endif
  	if (x) {
! #ifdef WIN32
!           /*
!            * Crummy hack. Need to figure out whether we have
!            * been redirected to a URL or to a file on some
!            * drive. Since I know of no protocols that are a
!            * single letter, if the : is the second character,
!            * I will assume a file was specified
!            */
!           if (strchr(x+2, ':')) {
! #else
! 	  if (strchr(x, ':')) {
! #endif /* WIN32 */
  	    redirect = pstrcat(r->pool, x, w, userdir, dname, NULL);
  	    table_set (r->headers_out, "Location", redirect);
  	    return REDIRECT;
--- 142,150 ----
        if (strchr(userdir, '*'))
  	x = getword(r->pool, &userdir, '*');
  
!       if ((ap_is_path_absolute(userdir)) || (userdir[0] == '\0')) {
  	if (x) {
!           if (ap_guess_if_path_is_uri(x)) {
  	    redirect = pstrcat(r->pool, x, w, userdir, dname, NULL);
  	    table_set (r->headers_out, "Location", redirect);
  	    return REDIRECT;



Re: OS abstraction

Posted by Dean Gaudet <dg...@arctic.org>.
On Mon, 23 Jun 1997, Paul Sutton wrote:
> Yes, sounds good. Do you mean use "inline" under GCC?

Yeah but it's a bit tricky to do it portably, and have function definitions
in only one place.  Here's an example:

inline.c:

/* this file should contain inlinable functions only */

#ifndef INLINE
#define INLINE
#include "whatever is needed"
#endif

INLINE int ap_thing (int whatever)
{
    blahblah;
    return thing;
}


And then in the appropriate .h file:

#if defined(__GNUC__) && !defined(INLINE)
#define INLINE extern inline
#include "inline.c"
#endif

It may look ugly at first, it's preprocessor play, but the goals are:

- support compilers without inline
- function definitions in only one place
- must provide a non-inlined version of each function (makes linking a
    debugging version a lot easier -- where inlining isn't used)
- avoid infinite #include recursion

If including a .c file scares you, then name it .h, and have a tiny .c that
essentially only includes the .h...

That enough to go on?  I can provide an example.

Dean


Re: OS abstraction

Posted by Paul Sutton <pa...@ukweb.com>.
On Sun, 22 Jun 1997, Dean Gaudet wrote:
> On Fri, 20 Jun 1997, Paul Sutton wrote:
> > os-unix.h
> >  #define AP_FILE_CREATE_PERMS \
> >               S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH
> >  #define ap_pipe(fds) pipe(fds)
> >  #define ap_is_path_absolute(file) (file && file[0] == '/' ? 1 : 0)
> >  #define ap_guess_if_path_is_uri(x) (strchr(x, ':')!= NULL)
> 
> I always get concerned about inadvertant use of a param with a side-effect
> in a macro ... like what if file there has a side effect.  I'd prefer
> this to be one of those cases where we use some of gcc's extensions if
> __GNUC__ and otherwise put it in a function.

Yes, sounds good. Do you mean use "inline" under GCC?

So this means we need an abstraction layer .c file as well as a .h.  So we
can get started, does anyone object to the following proposal (some of
this has already been discussed): 

  - create an abstraction directory (/src/os)

  - inside here, create a directory for each abstraction
     (/src/os/unix, /src/os/emx, /src/os/win32)

  - in each of those directories create a header file (os.h) for
     OS-specific macros (AP_*, some HAVE_*)

  - in each of these directories create a source file (os.c) for
     OS-specific implementations of ap_*() functions

  - move any other sources files tied completed to a single platform
     into the appropriate directory (e.g. Win32's service.c)

  - add code to conf.h to #include the appropriate /src/os/*/os.h file

We have a directory per platform (which Unix all lumped together).  When
this is all done we can start abstracting the OS specific stuff.

//pcs


Re: OS abstraction

Posted by Dean Gaudet <dg...@arctic.org>.
On Fri, 20 Jun 1997, Paul Sutton wrote:
> I think it would be nice to remove, as far as possible, the #ifdef __EMX__
> and #ifdef WIN32 sections from the core Apache core. 

Yup I listed that on that "nt todo list" I posted after reviewing the
cleanup.

> os-unix.h
> 
>  #define AP_FILE_CREATE_PERMS \
>               S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH
>  #define ap_pipe(fds) pipe(fds)
>  #define ap_is_path_absolute(file) (file && file[0] == '/' ? 1 : 0)
>  #define ap_guess_if_path_is_uri(x) (strchr(x, ':')!= NULL)

I always get concerned about inadvertant use of a param with a side-effect
in a macro ... like what if file there has a side effect.  I'd prefer
this to be one of those cases where we use some of gcc's extensions if
__GNUC__ and otherwise put it in a function.

> Finally I'll enclose a patch which shows how these abstractions make the
> code a bit neater (in my opinion anyway).

+1

Dean