You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Moenich <mo...@us.ibm.com> on 1999/04/09 15:23:13 UTC
[PATCH] TPF port
We're submitting a patch for the IBM/TPF operating system port. It's
been reviewed by IBM in Raleigh so we'd like your comments and review
also.
Two things to note about the TPF system that varies from typical UNIX:
fork() : fork processing on TPF does not pass the all the environment
from the parent so you'll notice initialization occurring in the child.
signals() : TPF is typically not interrupted by signals, so we
explicitly check for them. I hope we've abstracted it enough not to
affect other operating systems.
Thanks
Joe Moenich
moenich@us.ibm.com
moejoe@tde.com
303 773 5483
Re: [PATCH] TPF port
Posted by Martin Kraemer <Ma...@mch.sni.de>.
On Fri, Apr 09, 1999 at 07:23:13AM -0600, Joe Moenich wrote:
> we'd like your comments and review also.
> Index: apache/src/include/ap_config.h
> ===================================================================
> RCS file: /apache/apache/src/include/ap_config.h,v
> retrieving revision 1.1.1.1
> diff -u -a -b -r1.1.1.1 ap_config.h
> --- ap_config.h 1999/04/07 13:24:31 1.1.1.1
> +++ ap_config.h 1999/04/07 13:26:54
> @@ -996,6 +1006,11 @@
> #define strftime(s,max,format,tm) os_strftime(s,max,format,tm)
> #endif
> #include <signal.h>
> +#ifdef TPF
> +#ifdef NSIG
> +#undef NSIG
> +#endif
> +#endif
Why not #if defined(TPF) && defined(NSIG) ? Then you need only one
#endif and make the code clearer.
> @@ -1098,15 +1113,27 @@
> +#ifdef USE_TPF_SELECT
> +#define ap_select tpf_select
> +#elif defined(SELECT_NEEDS_CAST)
> #define ap_select(_a, _b, _c, _d, _e) \
> select((_a), (int *)(_b), (int *)(_c), (int *)(_d), (_e))
> #else
> #define ap_select select
> #endif
>
> +#ifdef USE_TPF_ACCEPT
> +#define ap_accept tpf_accept
> +#else
> +#define ap_accept accept
> +#endif
> +
> +#ifdef NEED_SIGNAL_INTERRUPT
> +#define ap_check_signals tpf_process_signals
> +#else
> +#define ap_check_signals
> +#endif
Uh oh! Bad style! If you want to redefine a function to another
function (or even to nothing) you better use a function-like macro.
Imagine what will happen when NEED_SIGNAL_INTERRUPT is not defined,
and the code in buff.c needs to compile the line...
ap_check_signals();
What it will see is:
();
and I doubt the compiler will make anythink smart out of it. Anyway,
it's always better to use a function-like makro in these cases as it
allows the preprocessor to check for the correct number of arguments
(this only brings you into trouble if you redefine before the actual
function declaration, and that function declaration is K&R style).
So better use something like
> +#define ap_select(_a, _b, _c, _d, _e) tpf_select(_a, _b, _c, _d, _e)
> +#define ap_check_signals() tpf_process_signals()
> +#define ap_accept(_fd, _sa, _ln) tpf_accept(_fd, _sa, _ln)
and similar.
> Index: apache/src/include/scoreboard.h
> ===================================================================
> RCS file: /apache/apache/src/include/scoreboard.h,v
> retrieving revision 1.1.1.1
> diff -u -a -b -r1.1.1.1 scoreboard.h
> --- scoreboard.h 1999/04/07 13:24:31 1.1.1.1
> +++ scoreboard.h 1999/04/07 13:27:02
> @@ -183,6 +183,8 @@
> } scoreboard;
>
> #define SCOREBOARD_SIZE sizeof(scoreboard)
> +#define SCOREBOARD_NAME "SCOREBRD"
> +#define SCOREBOARD_FRAMES SCOREBOARD_SIZE/4095 + 1
This makes it look like SCOREBOARD_NAME and SCOREBOARD_FRAMES were
some values generally important for apache, while in fact they're
only used for TPF. Please add an #ifdef around it (and optionally
add a comment).
> Index: apache/src/main/buff.c
> ===================================================================
> RCS file: /apache/apache/src/main/buff.c,v
> retrieving revision 1.1.1.1
> diff -u -a -b -r1.1.1.1 buff.c
> --- buff.c 1999/04/07 13:24:31 1.1.1.1
> +++ buff.c 1999/04/07 13:27:25
> @@ -263,7 +263,7 @@
> - tpf_process_signals();
> + ap_check_signals();
See note above
> Index: apache/src/main/http_main.c
> ===================================================================
> RCS file: /apache/apache/src/main/http_main.c,v
> retrieving revision 1.1.1.1
> diff -u -a -b -r1.1.1.1 http_main.c
> --- http_main.c 1999/04/07 13:24:31 1.1.1.1
> +++ http_main.c 1999/04/07 13:27:46
> @@ -3142,7 +3220,7 @@
> #endif
> ap_unblock_alarms();
>
> -#ifndef WIN32
> +#if !defined(WIN32) & !defined(TPF)
While not exactly incorrect, I'd prefer the logical comparison with
"&&" rather than the bitwise "&"
> @@ -3738,11 +3821,22 @@
> case ENETUNREACH:
> #endif
> break;
> -
> +#ifdef TPF
> + case EINACT:
> + ap_log_error(APLOG_MARK, APLOG_EMERG, server_conf,
> + "offload device inactive");
> + clean_child_exit(1);
> + break;
Is there any sense in letting the main server continue at this point?
Would clean_child_exit(APEXIT_CHILDFATAL); be more appropriate?
(Ehm... I don't know what an offload device is; isn't it like a swap
file?)
> @@ -4369,6 +4504,11 @@
> }
>
> perform_idle_server_maintenance();
> +#ifdef TPF
> + shutdown_pending = os_check_server(tpf_server_name);
> + ap_check_signals();
> + sleep(1);
> +#endif /*TPF */
Isn't the scoreboard maintenance delay handled by....
tv.tv_sec = SCOREBOARD_MAINTENANCE_INTERVAL / 1000000;
tv.tv_usec = SCOREBOARD_MAINTENANCE_INTERVAL % 1000000;
ap_select(0, NULL, NULL, NULL, &tv);
..in http_main? Why is there an additional sleep(1)? And why isn't
it sleep(SCOREBOARD_MAINTENANCE_INTERVAL / 1000000)?
> Index: apache/src/os/tpf/os.c
> ===================================================================
> RCS file: /apache/apache/src/os/tpf/os.c,v
> retrieving revision 1.1.1.1
> diff -u -a -b -r1.1.1.1 os.c
> --- os.c 1999/04/07 13:24:33 1.1.1.1
> +++ os.c 1999/04/07 13:28:13
> + if (r->method_number == M_PUT)
> + ap_bsetflag(r->connection->client, B_ASCII2EBCDIC, 0);
I'm not sure if this is correct here. The decision whether input
conversion has to occur must be based on the Content-Type of the PUT
contents, not on the Content-Type of the returned output. So it must
happen after the (EBCDIC converted) header lines of the PUT body
have been read.
> +#ifndef __PIPE_
Two '_' in front, but only one in the tail?
Cheers,
Martin
--
<Ma...@MchP.Siemens.De> | Siemens Information and
Phone: +49-89-636-46021 | Communication Products
FAX: +49-89-636-47816 | 81730 Munich, Germany
Re: [PATCH] TPF port
Posted by Dean Gaudet <dg...@arctic.org>.
> @@ -3060,8 +3138,8 @@
> }
> #endif /*_OSD_POSIX*/
> one = 1;
> -#ifndef BEOS
> -/* BeOS does not support SO_KEEPALIVE */
> +#if !defined(BEOS) && !defined(TPF)
> +/* BeOS and TPF do not support SO_KEEPALIVE */
> if (setsockopt(s, SOL_SOCKET, SO_KEEPALIVE, (char *) &one, sizeof(int)) < 0) {
> ap_log_error(APLOG_MARK, APLOG_CRIT, server_conf,
> "make_sock: for %s, setsockopt: (SO_KEEPALIVE)", addr);
Why don't we just change that to:
#ifdef SO_KEEPALIVE
if (setsockopt(s, SOL_SOCKET, SO_KEEPALIVE, ...
...
#endif
> +#if !defined(WIN32) & !defined(TPF)
> /* protect various fd_sets */
> if (s >= FD_SETSIZE) {
> ap_log_error(APLOG_MARK, APLOG_NOERRNO|APLOG_WARNING, NULL,
smells like something we may want to have a common define for:
CHECK_FD_SETSIZE
or something like that.
... http_main.c is such a mess of #ifdefs, eh?
Dean