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/10/16 22:38:45 UTC
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix networkio.h
sockets.c sockopt.c
On 16 Oct 1999 rbb@hyperreal.org wrote:
>...
> 1.3 +55 -7 apache-2.0/src/lib/apr/inc/apr_macro.h
>
> Index: apr_macro.h
> ===================================================================
> RCS file: /home/cvs/apache-2.0/src/lib/apr/inc/apr_macro.h,v
> retrieving revision 1.2
> retrieving revision 1.3
> diff -u -r1.2 -r1.3
> --- apr_macro.h 1999/10/16 12:47:59 1.2
> +++ apr_macro.h 1999/10/16 14:06:59 1.3
> @@ -62,18 +62,20 @@
> extern "C" {
> #endif
>
> +#include "apr_config.h"
> +
> #ifndef _POSIX_THREAD_SAFE_FUNCTIONS
> -#define SAFETY_LOCK(func_name, name_str) \
> +#define SAFETY_LOCK(funcname, namestr) \
> { \
> if (lock_##func_name == NULL) \
> if (ap_create_lock(&lock_##func_name, APR_MUTEX, APR_INTRAPROCESS, \
Did you try compiling? It looks like you missed two func_name values here.
> name_str, NULL) != APR_SUCCESS) \
> return APR_NOTTHREADSAFE; \
> - if (ap_lock(lock_##func_name) != APR_SUCCESS) \
> + if (ap_lock(lock_##funcname) != APR_SUCCESS) \
>...
> @@ -86,15 +88,61 @@
> #endif
>
> #ifdef HAVE_GMTIME_R
> -#define GMTIME_R(x, y) gmtime_r(x, y)
> +#define GMTIME_R(x, y) gmtime_r(x, y);
> #else
> -#define GMTIME_R(x, y) memcpy(y, gmtime(x), sizeof(y))
> +#define GMTIME_R(x, y) memcpy(y, gmtime(x), sizeof(y));
> #endif
Ack!
Ryan, if you're trying to create a macro that has "statement" like
binding, then the above is really not the way to do it. Consider the
following code:
if (expr)
GMTIME_R(x,y);
else
whatever;
The above will be seriously broken -- there are *two* semicolons in there.
Therefore, the second semicolon terminates the if/else. The compiler will
then see the second "else" and either fail with a syntax error, or (even
worse) bind it to an "if" further up.
There are a couple idioms for doing "statement" macros that I've seen:
#define FOO(x) if (1) { do_your_stuff(x); } else
#define BAR(x) do { do_your_stuff(x); } while (0)
Both of these are a proper statement and the semicolon people will use
simply terminates the if/do statement.
>...
> +#ifdef _POSIX_THREAD_SAFE_FUNCTIONS
> +#define GETHOSTBYNAME(x, y, z) z = gethostbyname(x);
> +#else
> +#define GETHOSTBYNAME(x, y, z) \
> + y = gethostbyname(x); \
> + z = ap_palloc(NULL, sizeof(struct hostent)); \
> + memcpy(z, y, sizeof(struct hostent));
> +#endif
The above should be in an if/do so that the statement are properly
combined. The above would fail spectacularly in an if/else where the
person didn't put braces in (like my original example).
> +#ifdef _POSIX_THREAD_SAFE_FUNCTIONS
> +#define GETHOSTBYADDR(x, y, z) z = gethostbyaddr(x, sizeof(struct in_addr), AF_INET);
> +#else
> +#define GETHOSTBYADDR(x, y, z) \
> + y = gethostbyaddr(x, sizeof(struct in_addr), AF_INET); \
> + z = ap_palloc(NULL, sizeof(struct hostent)); \
> + memcpy(z, y, sizeof(struct hostent));
> +#endif
if/do here, too.
> +#ifdef _POSIX_THREAD_SAFE_FUNCTIONS
> +#define INET_NTOA(x, y, len) ap_cpystrn(y, inet_ntoa(x), len);
> +#else
> +#define INET_NTOA(x, y, len) ap_cpystrn(y, inet_ntoa(x), len);
> +#endif
Why are these two conditions the same? Do you imagine they would be
different at some point? Did you just skip writing the full logic for this
one now, to come back to it later?
> +#ifdef _POSIX_THREAD_SAFE_FUNCTIONS
> +#define READDIR(x, y, z) y = readdir(x); \
> + if (y == NULL) { \
> + z = errno; \
> + } \
> + else { \
> + z = APR_SUCCESS; \
> + }
> #else
> -#define LOCALTIME_R(x, y) memcpy(y, localtime(x), sizeof(y))
> +#define READDIR(x, y, z) \
> + { \
> + struct dirent *temp = readdir(x); \
> + if (temp == NULL) { \
> + z = errno; \
> + } \
> + else { \
> + memcpy(y, temp, sizeof(struct dirent)); \
> + z = APR_SUCCESS; \
> + } \
> + }
Definitely needs an if/do to properly scope this stuff.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix networkio.h
sockets.c sockopt.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
Just to clarify. I'm not going to fix this. I would rather do this "the
right way", then work on a method I think is a dead end. If somebody
wants to fix the macros, go ahead, but I have decided to write new
functions to handle the non-thread safe functions, rather than try locking
them.
Ryan
On Mon, 18 Oct 1999, Dean Gaudet wrote:
> fwiw, i prefer "do { stuff } while (0)" ... and that's the technique
> currently in use in various places within apache.
>
> Dean
>
> On Sat, 16 Oct 1999, Greg Stein wrote:
>
> > Manoj Kasichainula wrote:
> > >
> > > On Sat, Oct 16, 1999 at 01:38:45PM -0700, Greg Stein wrote:
> > > > There are a couple idioms for doing "statement" macros that I've seen:
> > > >
> > > > #define FOO(x) if (1) { do_your_stuff(x); } else
> > > > #define BAR(x) do { do_your_stuff(x); } while (0)
> > >
> > > I've generally used:
> > >
> > > #define SHMUP(x) {do_your_stuff(x);}
> > >
> > > Do some compilers not allow bare braced statement blocks?
> >
> > Compilers will allow that, but it still doesn't bind properly. Look at
> > the following:
> >
> > if (foo)
> > SHMUP(bar);
> > else
> > baz;
> >
> > This would expand to:
> >
> > if (foo)
> > {do_your_stuff(bar);};
> > else
> > baz;
> >
> > Note the extra semicolon. However, if you use the approach that I
> > suggested, then you end up with:
> >
> > if (1) { do_your_stuff(x); } else ;
> >
> > The "else" statement block is a single empty statement -- the semicolon.
> > Therefore, it completes the (inner) if/else statement. The inner if/else
> > is the "if" statement, while "baz;" is the "else" statement.
> >
> > The issue generally boils down to people putting a semicolon after a
> > macro that looks like a function. This is usually for a few reasons:
> > habit, style, and auto-indent. Your form would be fine without the
> > semicolon and Ryan's would, too (except for the multi-statement things;
> > those would still break in certain if/else scenarios). Because of the
> > habit/style/autoindent issue, though, I strongly urge the use of the
> > if/do idioms in any macro that acts as a statement (rather than an
> > expression).
> >
> > Cheers,
> > -g
> >
> > --
> > Greg Stein, http://www.lyra.org/
> >
>
_______________________________________________________________________
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/network_io/unix networkio.h
sockets.c sockopt.c
Posted by Dean Gaudet <dg...@arctic.org>.
fwiw, i prefer "do { stuff } while (0)" ... and that's the technique
currently in use in various places within apache.
Dean
On Sat, 16 Oct 1999, Greg Stein wrote:
> Manoj Kasichainula wrote:
> >
> > On Sat, Oct 16, 1999 at 01:38:45PM -0700, Greg Stein wrote:
> > > There are a couple idioms for doing "statement" macros that I've seen:
> > >
> > > #define FOO(x) if (1) { do_your_stuff(x); } else
> > > #define BAR(x) do { do_your_stuff(x); } while (0)
> >
> > I've generally used:
> >
> > #define SHMUP(x) {do_your_stuff(x);}
> >
> > Do some compilers not allow bare braced statement blocks?
>
> Compilers will allow that, but it still doesn't bind properly. Look at
> the following:
>
> if (foo)
> SHMUP(bar);
> else
> baz;
>
> This would expand to:
>
> if (foo)
> {do_your_stuff(bar);};
> else
> baz;
>
> Note the extra semicolon. However, if you use the approach that I
> suggested, then you end up with:
>
> if (1) { do_your_stuff(x); } else ;
>
> The "else" statement block is a single empty statement -- the semicolon.
> Therefore, it completes the (inner) if/else statement. The inner if/else
> is the "if" statement, while "baz;" is the "else" statement.
>
> The issue generally boils down to people putting a semicolon after a
> macro that looks like a function. This is usually for a few reasons:
> habit, style, and auto-indent. Your form would be fine without the
> semicolon and Ryan's would, too (except for the multi-statement things;
> those would still break in certain if/else scenarios). Because of the
> habit/style/autoindent issue, though, I strongly urge the use of the
> if/do idioms in any macro that acts as a statement (rather than an
> expression).
>
> Cheers,
> -g
>
> --
> Greg Stein, http://www.lyra.org/
>
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix networkio.h sockets.c sockopt.c
Posted by Greg Stein <gs...@lyra.org>.
Manoj Kasichainula wrote:
>
> On Sat, Oct 16, 1999 at 01:38:45PM -0700, Greg Stein wrote:
> > There are a couple idioms for doing "statement" macros that I've seen:
> >
> > #define FOO(x) if (1) { do_your_stuff(x); } else
> > #define BAR(x) do { do_your_stuff(x); } while (0)
>
> I've generally used:
>
> #define SHMUP(x) {do_your_stuff(x);}
>
> Do some compilers not allow bare braced statement blocks?
Compilers will allow that, but it still doesn't bind properly. Look at
the following:
if (foo)
SHMUP(bar);
else
baz;
This would expand to:
if (foo)
{do_your_stuff(bar);};
else
baz;
Note the extra semicolon. However, if you use the approach that I
suggested, then you end up with:
if (1) { do_your_stuff(x); } else ;
The "else" statement block is a single empty statement -- the semicolon.
Therefore, it completes the (inner) if/else statement. The inner if/else
is the "if" statement, while "baz;" is the "else" statement.
The issue generally boils down to people putting a semicolon after a
macro that looks like a function. This is usually for a few reasons:
habit, style, and auto-indent. Your form would be fine without the
semicolon and Ryan's would, too (except for the multi-statement things;
those would still break in certain if/else scenarios). Because of the
habit/style/autoindent issue, though, I strongly urge the use of the
if/do idioms in any macro that acts as a statement (rather than an
expression).
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix networkio.h sockets.c sockopt.c
Posted by Manoj Kasichainula <ma...@io.com>.
On Sat, Oct 16, 1999 at 01:38:45PM -0700, Greg Stein wrote:
> There are a couple idioms for doing "statement" macros that I've seen:
>
> #define FOO(x) if (1) { do_your_stuff(x); } else
> #define BAR(x) do { do_your_stuff(x); } while (0)
I've generally used:
#define SHMUP(x) {do_your_stuff(x);}
Do some compilers not allow bare braced statement blocks?
--
Manoj Kasichainula - manojk at io dot com - http://www.io.com/~manojk/
"Hypocrisy is the greatest luxury." - Disposable Heroes of Hiphoprisy
Re: cvs commit: apache-2.0/src/lib/apr/network_io/unix networkio.h
sockets.c sockopt.c
Posted by Ryan Bloom <rb...@raleigh.ibm.com>.
> > +
> > #ifndef _POSIX_THREAD_SAFE_FUNCTIONS
> > -#define SAFETY_LOCK(func_name, name_str) \
> > +#define SAFETY_LOCK(funcname, namestr) \
> > { \
> > if (lock_##func_name == NULL) \
> > if (ap_create_lock(&lock_##func_name, APR_MUTEX, APR_INTRAPROCESS, \
>
> Did you try compiling? It looks like you missed two func_name values here.
Not only did I compile, I also RAN the code multiple times. I don't know
how the compiler missed this. But, I am tending to think it has something
to do with both David Reid and I fixing some problems in this file in
different ways. Committing this change soon.
>
> There are a couple idioms for doing "statement" macros that I've seen:
>
> #define FOO(x) if (1) { do_your_stuff(x); } else
> #define BAR(x) do { do_your_stuff(x); } while (0)
>
> Both of these are a proper statement and the semicolon people will use
> simply terminates the if/do statement.
>
> if/do here, too.
>
All fixed. Committing RSN. Just waiting for the compile to finish. :).
Teach me to rush through this stuff while thinking about something else.
I actually changed all of this just before committing. Oh well.
> > +#ifdef _POSIX_THREAD_SAFE_FUNCTIONS
> > +#define INET_NTOA(x, y, len) ap_cpystrn(y, inet_ntoa(x), len);
> > +#else
> > +#define INET_NTOA(x, y, len) ap_cpystrn(y, inet_ntoa(x), len);
> > +#endif
>
> Why are these two conditions the same? Do you imagine they would be
> different at some point? Did you just skip writing the full logic for this
> one now, to come back to it later?
These are the same, because I'm not sure what the correct solution is.
Single UNIX says inet_ntoa doesn't have to be thread-safe, but if you look
at what it is doing, it is most likely thread-safe. Because I couldn't
find a function that is definately thread-safe, I decided to just leave
this alone until I am near my books, which are all in the office, and
which might have the actual answer. BTW, this is the function, which lead
me to believe that we were cheating the issue, and we need to re-write all
the non-thread-safe functions.
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.