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/07 01:30:02 UTC
Re: cvs commit: apache-2.0/src/lib/apr/threadproc/unix thread.c
On 6 Dec 1999 rbb@hyperreal.org wrote:
> rbb 99/12/06 07:47:46
>
> Modified: src/lib/apr acconfig.h configure.in
> src/lib/apr/include apr.h.in apr_network_io.h
> src/lib/apr/lib apr_pools.c
> src/lib/apr/threadproc/unix thread.c
> Log:
> Cleanup some mistakes I made. We are now configuring the APR_HAS_FOO
> macros in a cleaner way IMO, and that required that we always use #if
> instead of #ifdef. I also used the wrong #if HAVE_SIGNAL_H macro
> in apr_pools.c.
This seems very wrong, to me. We just got done having a discussion that we
would be using the #ifdef variety for feature testing. I do not think we
ought to be using #ifdef for autoconf tests, but #if for APR tests. We are
sure to run into a confusion somewhere and have unexpected results.
Why can't we use #ifdef for APR, too?
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/threadproc/unix thread.c
Posted by David Reid <ab...@dial.pipex.com>.
I think Ryan was being economical with letters...
<from apr_pools.c>
#ifdef HAVE_SYS_SIGNAL_H
#include <sys/signal.h>
#endif
#ifdef HAVE_SIGNAL_H
#include <signal.h>
#endif
d.
----- Original Message -----
From: "Greg Stein" <gs...@lyra.org>
To: <ne...@apache.org>
Sent: 07 December 1999 00:30
Subject: Re: cvs commit: apache-2.0/src/lib/apr/threadproc/unix thread.c
> On 6 Dec 1999 rbb@hyperreal.org wrote:
> > rbb 99/12/06 07:47:46
> >
> > Modified: src/lib/apr acconfig.h configure.in
> > src/lib/apr/include apr.h.in apr_network_io.h
> > src/lib/apr/lib apr_pools.c
> > src/lib/apr/threadproc/unix thread.c
> > Log:
> > Cleanup some mistakes I made. We are now configuring the APR_HAS_FOO
> > macros in a cleaner way IMO, and that required that we always use #if
> > instead of #ifdef. I also used the wrong #if HAVE_SIGNAL_H macro
> > in apr_pools.c.
>
> This seems very wrong, to me. We just got done having a discussion that we
> would be using the #ifdef variety for feature testing. I do not think we
> ought to be using #ifdef for autoconf tests, but #if for APR tests. We are
> sure to run into a confusion somewhere and have unexpected results.
>
> Why can't we use #ifdef for APR, too?
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
>
Re: cvs commit: apache-2.0/src/lib/apr/threadproc/unix thread.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> > Because of the separation that autoconf requires. This code is MUCH
> > cleaner for the configuration. I can define shell variables to be 0/1 or
> > I can define them to be "#define APR_HAS_THREADS 1"/"#under
> > APR_HAS_THREADS", and then substitute those shell variables into a header
> > file. The first case is easier to understand when you look at the
> > pre-generated header file. I personally believe Apache will end up
> > heading this way as well, because it will need to do the same basic steps
> > as I have already done to get the separation required.
>
> You could also define shell variables as 0/1 and a utility macro/function
> spits out the correct form.
>
This comes down to APR and Apache being two different packages that happen
to be being written by the same group. They are no more related than
Apache and PHP are related in my mind, except that Apache is going to rely
on APR for it's OS specific code. Much in the same way that Apache relies
on PHP for a scripting language. The decisions I am making are the
correct decisions as I see them for APR. I am the person who gets
questions when APR doesn't compile on a given platform, and I am trying to
make my life and the life of any future APR person easier. I will not
change what is right for APR because it makes Apache's life easier.
In answer to your question, your solution doesn't address the problem.
When I look at apr.h, I can see one of two things.
@threads@
or
#define APR_HAS_THREADS @threads@
Now, the shell variable threads can expand to either 1/0 or "#define
APR_HAS_THREADS 1"/"#under APR_HAS_THREADS". As somebody who has to deal
with porting APR to other platforms, the second option makes it VERY
obvious what we are trying to define here. Think of this as a windows
programmer, who doesn't have autoconf, @threads@ could be just about
anything, but the second option makes it MUCH clearer what we are trying
to get across.
> Really: using two forms feels *very* wrong and subject to biting us in the
> ass down the road. "oh! that was a #if form rather than #ifdef! damn!"
The fact remains, this is the correct decision for APR. I really believe
that as we continue to use autoconf/automake in Apache, we are going to
find out that always using #ifdef is the wrong answer, because there are
going to be times that we actually do want to assign a 0/1/2/3 value to a
variable.
If you want to change so that all macros are using the same syntax,
re-visit the question in the Apache space. I personally don't care what
Apache uses, for all I care, in one of the Apache header files, we could
put
#if APR_HAS_MMAP
#define USE_MMAP_FILES
#else
#undef USE_MMAP_FILES
#endif
In fact, I did something like that just Friday. It gives the Apache
programmer a common syntax while still allowing APR to make the correct
decision for that project.
Until I see a reason other than "This isn't what we decided for Apache", I
don't consider this an open question.
Ryan
_______________________________________________________________________
Ryan Bloom rbb@raleigh.ibm.com
4205 S Miami Blvd
RTP, NC 27709 It's a beautiful sight to see good dancers
doing simple steps. It's a painful sight to
see beginners doing complicated patterns.
Re: cvs commit: apache-2.0/src/lib/apr/threadproc/unix thread.c
Posted by Greg Stein <gs...@lyra.org>.
On Tue, 7 Dec 1999 rbb@apache.org wrote:
> On Mon, 6 Dec 1999, Greg Stein wrote:
> > > Cleanup some mistakes I made. We are now configuring the APR_HAS_FOO
> > > macros in a cleaner way IMO, and that required that we always use #if
> > > instead of #ifdef. I also used the wrong #if HAVE_SIGNAL_H macro
> > > in apr_pools.c.
> >
> > This seems very wrong, to me. We just got done having a discussion that we
> > would be using the #ifdef variety for feature testing. I do not think we
> > ought to be using #ifdef for autoconf tests, but #if for APR tests. We are
> > sure to run into a confusion somewhere and have unexpected results.
> >
> > Why can't we use #ifdef for APR, too?
>
> Because of the separation that autoconf requires. This code is MUCH
> cleaner for the configuration. I can define shell variables to be 0/1 or
> I can define them to be "#define APR_HAS_THREADS 1"/"#under
> APR_HAS_THREADS", and then substitute those shell variables into a header
> file. The first case is easier to understand when you look at the
> pre-generated header file. I personally believe Apache will end up
> heading this way as well, because it will need to do the same basic steps
> as I have already done to get the separation required.
You could also define shell variables as 0/1 and a utility macro/function
spits out the correct form.
Really: using two forms feels *very* wrong and subject to biting us in the
ass down the road. "oh! that was a #if form rather than #ifdef! damn!"
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/threadproc/unix thread.c
Posted by rb...@apache.org.
On Mon, 6 Dec 1999, Greg Stein wrote:
> > Cleanup some mistakes I made. We are now configuring the APR_HAS_FOO
> > macros in a cleaner way IMO, and that required that we always use #if
> > instead of #ifdef. I also used the wrong #if HAVE_SIGNAL_H macro
> > in apr_pools.c.
>
> This seems very wrong, to me. We just got done having a discussion that we
> would be using the #ifdef variety for feature testing. I do not think we
> ought to be using #ifdef for autoconf tests, but #if for APR tests. We are
> sure to run into a confusion somewhere and have unexpected results.
>
> Why can't we use #ifdef for APR, too?
Because of the separation that autoconf requires. This code is MUCH
cleaner for the configuration. I can define shell variables to be 0/1 or
I can define them to be "#define APR_HAS_THREADS 1"/"#under
APR_HAS_THREADS", and then substitute those shell variables into a header
file. The first case is easier to understand when you look at the
pre-generated header file. I personally believe Apache will end up
heading this way as well, because it will need to do the same basic steps
as I have already done to get the separation required.
Ryan
_______________________________________________________________________________
Ryan Bloom rbb@ntrnet.net
6209 H Shanda Dr.
Raleigh, NC 27609 Ryan Bloom -- thinker, adventurer, artist,
writer, but mostly, friend.
-------------------------------------------------------------------------------