You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Greg Stein <gs...@lyra.org> on 1999/12/20 19:48:52 UTC

Re: cvs commit: apache-2.0/src/modules/standard mod_include.c mod_log_config.c mod_negotiation.c

On 20 Dec 1999 rbb@hyperreal.org wrote:
> rbb         99/12/20 08:38:41
> 
>   Modified:    src/include http_protocol.h http_request.h httpd.h
>                         util_date.h
>                src/main http_core.c http_protocol.c http_request.c util.c
>                         util_date.c util_script.c
>                src/modules/standard mod_include.c mod_log_config.c
>                         mod_negotiation.c
>   Log:
>   First step in getting Apache to use APR's time libraries.  This gets a good
>   number of them, but I think there are more time values still in the Apache
>   code.  This works under Linux, but has not been tested anywhere else.
>...
>   --- httpd.h	1999/12/08 00:14:03	1.14
>   +++ httpd.h	1999/12/20 16:38:32	1.15
>   @@ -71,6 +71,7 @@
>    #include "ap_config.h"
>    #include "apr_general.h"
>    #include "apr_lib.h"
>   +#include "apr_time.h"
>    #include "buff.h"
>    #include "ap.h"

I thought you had said that the "proper procedure" was to just include
"apr.h", but here you include several different headers.

What is "proper"?

thx,
-g

-- 
Greg Stein, http://www.lyra.org/


Re: cvs commit: apache-2.0/src/modules/standard mod_include.c mod_log_config.c mod_negotiation.c

Posted by Manoj Kasichainula <ma...@io.com>.
(Text rearranged, and extra context provided, since I took so long to
respond to this)

On Tue, Dec 21, 1999 at 05:54:10PM -0500, Ryan Bloom wrote:
> Option 1)
> 
> In apr.h.in:
> 
> @THREADS@
> 
> In configure.in: (In psuedo-code)
> 
> if {check for threading support}
>     THREADS="#define APR_HAS_THREADS
> else
>     THREADS="#undef APR_HAS_THREADS
> fi
> 
> AC_SUBST(THREADS)
>
> I initially opted for the first method, because it stuck with the #if rule
> that Apache had adopted.  I happened to have made this change a few days
> before leaving work for a few days for personal reasons.  While I was
> gone, I had three different people contact me to find out what those
> @THREADS@ things were there for.  One of these people was trying to get
> autoconf working on his system again, the other two were trying to port
> APR to a platform without autoconf.

This probably would've been much clearer to people if the macro had
been DEFINE_IF_THREADS_ENABLED, with a comment mentioning what each
of these were. Then, I would think people wouldn't be confused.

Or:

Option 3)

(what PHP does for some vars it wants to expose)

configure.in:

PHP_VERSION=$VERSION
echo "/* automatically generated by configure */" > php_version.h.new
echo "/* edit configure.in.in to change version number */" >> php_version.h.new
echo "#define PHP_VERSION \"$PHP_VERSION\"" >> php_version.h.new

etc.

> Option 2)
> 
> In apr.h:
> 
> #define APR_HAS_THREADS @THREADS@
> 
> In configure.in
> 
> if {check for threading support}
>     THREADS="1"
> else
>     THREADS="0"
> fi
> 
> AC_SUBST(THREADS)
> 
> Option 2 is much clearer in this respect.  It is obvious what we are
> trying to figure out.  Plus, #if is much more powerful that #ifdef.  There
> are times that we don't want to check if something is defined, but we want
> to check what the value of a definition is.  Case in point, What version
> of AIX are we on?

Some autoconf macros' content are inspected. See the results of
AC_FUNC_SELECT_ARGTYPES, for example.

> If we are going to come right out and limit
> ourselves to either #if or #ifdef, them we had better choose wisely,
> because this issue will bite us when we want the added functionality of
> #if, but we can't use it because Apache 2.0 only uses #ifdef.

The autoheader convention is to #undef feature macros. And AFAIK, the
overwhelming possibility is that all ANSI compilers will support #if
if the feature macros are undefed.

So, I think that we should #undef feature macros, and we will have the
ability to convert #ifs to #ifdefs or back whenever we want.


Re: cvs commit: apache-2.0/src/modules/standard mod_include.c mod_log_config.c mod_negotiation.c

Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> Where that stands right now is that we know that #ifdef will work on
> all compilers when something is undefed, but we don't know this for
> #if. And since autoheader generates #undef/#define, I have no
> inclination to change the definition of those macros. If all compilers
> will not barf horribly on #if BLAH if BLAH isn't defined, then I have
> no qualms about switching to #if (though I'm too lazy to bother with
> it myself).
> 
> The positives and negatives of the two approaches should be the same
> for Apache and APR. Ideally, the solution should be as well.

The positives and negatives for APR are quite simple, and I have already
specified them once, I will do so again now.

In APR, we were forced to create an APR.H file, which defines all of the
exposed feature macros that APR has.  In this new file, I needed to define
macros like:

#define APR_HAS_THREADS
#define APR_HAS_MMAP

I had two possibilities for doing this.

Option 1)

In apr.h.in:

@THREADS@

In configure.in: (In psuedo-code)

if {check for threading support}
    THREADS="#define APR_HAS_THREADS
else
    THREADS="#undef APR_HAS_THREADS
fi

AC_SUBST(THREADS)
Option 2)

In apr.h:

#define APR_HAS_THREADS @THREADS@

In configure.in

if {check for threading support}
    THREADS="1"
else
    THREADS="0"
fi

AC_SUBST(THREADS)

I initially opted for the first method, because it stuck with the #if rule
that Apache had adopted.  I happened to have made this change a few days
before leaving work for a few days for personal reasons.  While I was
gone, I had three different people contact me to find out what those
@THREADS@ things were there for.  One of these people was trying to get
autoconf working on his system again, the other two were trying to port
APR to a platform without autoconf.

I consider all three of these people very intelligent and very good
programmers, but all three are also very busy and new to autoconf.  If
they couldn't figure out what the @THREADS@ type macros were, then how
could I reasonably expect a person, who is trying to port Apache 2.0 and
APR to a new platform, to be able to figure them out?  The only answer I
could come to was I couldn't expect it.

Option 2 is much clearer in this respect.  It is obvious what we are
trying to figure out.  Plus, #if is much more powerful that #ifdef.  There
are times that we don't want to check if something is defined, but we want
to check what the value of a definition is.  Case in point, What version
of AIX are we on?  In Apache 1.3.10, we modified the dlopen code on AIX,
so that if we are on 4.3 or later, we use AIX's provided dlopen, because
it has been fixed and it works for 64-bit programs.  If, however, we are
on AIX 4.2 or earlier, we use Apache's dlopen, because AIX's is broken.

I don't know how to make that check with #ifdef.  I do know how to check
if HAVE_MALLOC_H is defined or not with #if, just use #if
defined(HAVE_MALLOC_H).  If we are going to come right out and limit
ourselves to either #if or #ifdef, them we had better choose wisely,
because this issue will bite us when we want the added functionality of
#if, but we can't use it because Apache 2.0 only uses #ifdef.

But, we made our decision already.  :-)

Ryan




_______________________________________________________________________
Ryan Bloom		rbb@raleigh.ibm.com
4205 S Miami Blvd	
RTP, NC 27709		

Come to the first official Apache Software Foundation
Conference!  <http://ApacheCon.Com/>



Re: cvs commit: apache-2.0/src/modules/standard mod_include.c mod_log_config.c mod_negotiation.c

Posted by Manoj Kasichainula <ma...@io.com>.
On Tue, Dec 21, 1999 at 05:20:33PM -0500, Ryan Bloom wrote:
> I still think the #ifdef decision is wrong for Apache

Where that stands right now is that we know that #ifdef will work on
all compilers when something is undefed, but we don't know this for
#if. And since autoheader generates #undef/#define, I have no
inclination to change the definition of those macros. If all compilers
will not barf horribly on #if BLAH if BLAH isn't defined, then I have
no qualms about switching to #if (though I'm too lazy to bother with
it myself).

The positives and negatives of the two approaches should be the same
for Apache and APR. Ideally, the solution should be as well.


Re: cvs commit: apache-2.0/src/modules/standard mod_include.c mod_log_config.c mod_negotiation.c

Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> Agreed... I understand that part. Personally, I'd like to see some of
> those decisions come out in the same way (e.g. #if vs #ifdef), but hey...
> I'm not coding it, so I have no legs to stand on! :-)

Actually, as little as it looks like it, I am trying to keep the code
looking similar.  Most of the coding style rules are the same.  When it
comes to indentation, and other coding rules, I bowed to Apache's style.
I did this for a few reasons, 1...I like that style, 2... it makes the
code easy to transition between.

I still think the #ifdef decision is wrong for Apache, but that was the
groups decision, so I will stand by it.  But, in APR it just makes things
harder for those of us who have to port to different systems.

Ryan

_______________________________________________________________________
Ryan Bloom		rbb@raleigh.ibm.com
4205 S Miami Blvd	
RTP, NC 27709		

Come to the first official Apache Software Foundation
Conference!  <http://ApacheCon.Com/>



Re: cvs commit: apache-2.0/src/modules/standard mod_include.c mod_log_config.c mod_negotiation.c

Posted by Greg Stein <gs...@lyra.org>.
On Mon, 20 Dec 1999, Ryan Bloom wrote:
> Greg Stein wrote:
>...
> > I thought you had said that the "proper procedure" was to just include
> > "apr.h", but here you include several different headers.
> > 
> > What is "proper"?
> 
> Never said we should just be including apr.h.  apr.h doesn't include ANY
> other header files, it just gives us access to all of the APR feature
> macros that are namespace protected.

Oh. I think I got messed up in my thinking then. Heh. I know you said APR
clients shouldn't use "apr_config.h", but I mis-remembered you saying they
should only use "apr.h".

All righty. Programming pattern locked in and ready to use. :-)

> As far as including one header file goes, inside of APR it was decided to
> include the os specific header file, and let that header file include all
> of the system specific stuff.  This kept us from doing the following:

Makes total sense.

>...
> I would really like to stress that APR and Apache are two DIFFERENT
> projects in my mind.  What I do for one project doesn't always make sense
> for the other project.

Agreed... I understand that part. Personally, I'd like to see some of
those decisions come out in the same way (e.g. #if vs #ifdef), but hey...
I'm not coding it, so I have no legs to stand on! :-)

Happy Holidays,
-g

-- 
Greg Stein, http://www.lyra.org/


Re: cvs commit: apache-2.0/src/modules/standard mod_include.c mod_log_config.c mod_negotiation.c

Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> >   --- httpd.h	1999/12/08 00:14:03	1.14
> >   +++ httpd.h	1999/12/20 16:38:32	1.15
> >   @@ -71,6 +71,7 @@
> >    #include "ap_config.h"
> >    #include "apr_general.h"
> >    #include "apr_lib.h"
> >   +#include "apr_time.h"
> >    #include "buff.h"
> >    #include "ap.h"
> 
> I thought you had said that the "proper procedure" was to just include
> "apr.h", but here you include several different headers.
> 
> What is "proper"?

Never said we should just be including apr.h.  apr.h doesn't include ANY
other header files, it just gives us access to all of the APR feature
macros that are namespace protected.

As far as including one header file goes, inside of APR it was decided to
include the os specific header file, and let that header file include all
of the system specific stuff.  This kept us from doing the following:

In apr/file_io/unix/open.c

#include <stdio.h>
#include <sys/types.h>

in apr/file_io/unix/filedup.c

#include <stdio.h>
#include <sys/types.h>

and so on for the rest of the files in file_io/unix.

I would really like to stress that APR and Apache are two DIFFERENT
projects in my mind.  What I do for one project doesn't always make sense
for the other project.

In APR, for each sub-library, there is a small header file for a
particular platform, it is much easier to just include that one header
file in each C file.  In Apache, we have multiple header files that need
to be included in each C file, and I would much rather make it easy to
find functions, rather than chasing through header files to find all the
rest of the headers that have been included.

Ryan

_______________________________________________________________________
Ryan Bloom		rbb@raleigh.ibm.com
4205 S Miami Blvd	
RTP, NC 27709		

Come to the first official Apache Software Foundation
Conference!  <http://ApacheCon.Com/>