You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by je...@apache.org on 2004/11/28 20:37:42 UTC

svn commit: r106843 - /httpd/httpd/trunk/include/ap_listen.h /httpd/httpd/trunk/server/listen.c

Author: jerenkrantz
Date: Sun Nov 28 11:37:41 2004
New Revision: 106843

URL: http://svn.apache.org/viewcvs?view=rev&rev=106843
Log:
Properly export ap_listen_* functions.

* server/listen.c: Add AP_DECLARE as appropriate.
* include/ap_listen.h: Add AP_DECLARE as appropriate.

Modified:
   httpd/httpd/trunk/include/ap_listen.h
   httpd/httpd/trunk/server/listen.c

Modified: httpd/httpd/trunk/include/ap_listen.h
Url: http://svn.apache.org/viewcvs/httpd/httpd/trunk/include/ap_listen.h?view=diff&rev=106843&p1=httpd/httpd/trunk/include/ap_listen.h&r1=106842&p2=httpd/httpd/trunk/include/ap_listen.h&r2=106843
==============================================================================
--- httpd/httpd/trunk/include/ap_listen.h	(original)
+++ httpd/httpd/trunk/include/ap_listen.h	Sun Nov 28 11:37:41 2004
@@ -63,7 +63,7 @@
 /**
  * Setup all of the defaults for the listener list
  */
-void ap_listen_pre_config(void);
+AP_DECLARE(void) ap_listen_pre_config(void);
 #if !defined(SPMT_OS2_MPM)
 /**
  * Loop through the global ap_listen_rec list and create all of the required
@@ -73,7 +73,7 @@
  * @warning This function is not available to Windows platforms, or the
  * Prefork or SPMT_OS2 MPMs.
  */ 
-int ap_setup_listeners(server_rec *s);
+AP_DECLARE(int) ap_setup_listeners(server_rec *s);
 #endif
 /* Split into two #if's to make the exports scripts easier.
  */
@@ -87,7 +87,7 @@
  * @warning This function is only available to Windows platforms, or the
  * Prefork or SPMT_OS2 MPMs.
  */
-int ap_listen_open(process_rec *process, apr_port_t port);
+AP_DECLARE(int) ap_listen_open(process_rec *process, apr_port_t port);
 #endif
 
 /* Although these functions are exported from libmain, they are not really
@@ -97,9 +97,9 @@
  * LISTEN_COMMANDS in their command_rec table so that these functions are
  * called.
  */ 
-const char *ap_set_listenbacklog(cmd_parms *cmd, void *dummy, const char *arg);
-const char *ap_set_listener(cmd_parms *cmd, void *dummy, const char *ips);
-const char *ap_set_send_buffer_size(cmd_parms *cmd, void *dummy,
+AP_DECLARE(const char *) ap_set_listenbacklog(cmd_parms *cmd, void *dummy, const char *arg);
+AP_DECLARE(const char *) ap_set_listener(cmd_parms *cmd, void *dummy, const char *ips);
+AP_DECLARE(const char *) ap_set_send_buffer_size(cmd_parms *cmd, void *dummy,
 				    const char *arg);
 
 #define LISTEN_COMMANDS	\

Modified: httpd/httpd/trunk/server/listen.c
Url: http://svn.apache.org/viewcvs/httpd/httpd/trunk/server/listen.c?view=diff&rev=106843&p1=httpd/httpd/trunk/server/listen.c&r1=106842&p2=httpd/httpd/trunk/server/listen.c&r2=106843
==============================================================================
--- httpd/httpd/trunk/server/listen.c	(original)
+++ httpd/httpd/trunk/server/listen.c	Sun Nov 28 11:37:41 2004
@@ -28,7 +28,7 @@
 #include "mpm.h"
 #include "mpm_common.h"
 
-ap_listen_rec *ap_listeners = NULL;
+AP_DECLARE_DATA ap_listen_rec *ap_listeners = NULL;
 
 static ap_listen_rec *old_listeners;
 static int ap_listenbacklog;
@@ -288,7 +288,7 @@
     return NULL;
 }
 
-static int ap_listen_open(apr_pool_t *pool, apr_port_t port)
+AP_DECLARE(int) ap_listen_open(apr_pool_t *pool, apr_port_t port)
 {
     ap_listen_rec *lr;
     ap_listen_rec *next;
@@ -418,7 +418,7 @@
     return num_open ? 0 : -1;
 }
 
-int ap_setup_listeners(server_rec *s)
+AP_DECLARE(int) ap_setup_listeners(server_rec *s)
 {
     ap_listen_rec *lr;
     int num_listeners = 0;
@@ -434,7 +434,7 @@
     return num_listeners;
 }
 
-void ap_listen_pre_config(void)
+AP_DECLARE(void) ap_listen_pre_config(void)
 {
     old_listeners = ap_listeners;
     ap_listeners = NULL;
@@ -442,7 +442,7 @@
 }
 
 
-const char *ap_set_listener(cmd_parms *cmd, void *dummy, const char *ips)
+AP_DECLARE(const char *) ap_set_listener(cmd_parms *cmd, void *dummy, const char *ips)
 {
     char *host, *scope_id;
     apr_port_t port;
@@ -474,7 +474,7 @@
     return alloc_listener(cmd->server->process, host, port);
 }
 
-const char *ap_set_listenbacklog(cmd_parms *cmd, void *dummy, const char *arg)
+AP_DECLARE(const char *) ap_set_listenbacklog(cmd_parms *cmd, void *dummy, const char *arg)
 {
     int b;
     const char *err = ap_check_cmd_context(cmd, GLOBAL_ONLY);
@@ -492,7 +492,7 @@
     return NULL;
 }
 
-const char *ap_set_send_buffer_size(cmd_parms *cmd, void *dummy,
+AP_DECLARE(const char *) ap_set_send_buffer_size(cmd_parms *cmd, void *dummy,
                                     const char *arg)
 {
     int s = atoi(arg);

Re: svn commit: r106843 - /httpd/httpd/trunk/include/ap_listen.h /httpd/httpd/trunk/server/listen.c

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Monday, November 29, 2004 6:52 PM +0100 Mladen Turk <mt...@apache.org> 
wrote:

> Sorry its cmd_func not cmd_table.
>
> AP_DECLARE(const char *) ap_set_listenbacklog(...)
> evaluates to:
> __declspec(dllexport) const char * __stdcall ap_set_listenbacklog(...)
>
> Later on there is:
>
> AP_INIT_TAKE1("ListenBacklog", ap_set_listenbacklog, NULL, RSRC_CONF ...

Okay, so is the only one that should be declared AP_DECLARE_NONSTD 
listenbacklog?  None of the others seem to be used that way, AFAICT.  -- justin

Re: svn commit: r106843 - /httpd/httpd/trunk/include/ap_listen.h /httpd/httpd/trunk/server/listen.c

Posted by Mladen Turk <mt...@apache.org>.
Justin Erenkrantz wrote:
> On Mon, Nov 29, 2004 at 06:30:35PM +0100, Mladen Turk wrote:
> 
>>__stdcall: The callee cleans the stack
>>__cdecl (default): The stack is cleaned up by the caller
>>
>>So you can not mix that two.
>>The cmd_table uses __cdecl functions and you are providing
>>__stdcall function, so the build fails.
> 
> 
> You are still being obtuse.  I don't see how the ap_listen functions interact
> with the cmd_table (which are just the directives, right?).  What functions
> are declared as __stdcall that interact with this? 
> 

Sorry its cmd_func not cmd_table.

AP_DECLARE(const char *) ap_set_listenbacklog(...)
evaluates to:
__declspec(dllexport) const char * __stdcall ap_set_listenbacklog(...)

Later on there is:

AP_INIT_TAKE1("ListenBacklog", ap_set_listenbacklog, NULL, RSRC_CONF ...

cmd_func is declared as :

     const char *(*take1) (...)

And that is by defauld __cdecl.
When compiling mpm_wint you have three error for LISTEN_COMMANDS.

> 
> Without that patch, it doesn't work on any non-Win32 platforms.  I think my
> recent reorg of files somehow brought a latent build bug up.  -- justin
>

AP_DECLARE_NONSTD could be used thought, just add that to build_exports


MT.

Re: svn commit: r106843 - /httpd/httpd/trunk/include/ap_listen.h /httpd/httpd/trunk/server/listen.c

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
On Mon, Nov 29, 2004 at 06:30:35PM +0100, Mladen Turk wrote:
> __stdcall: The callee cleans the stack
> __cdecl (default): The stack is cleaned up by the caller
> 
> So you can not mix that two.
> The cmd_table uses __cdecl functions and you are providing
> __stdcall function, so the build fails.

You are still being obtuse.  I don't see how the ap_listen functions interact
with the cmd_table (which are just the directives, right?).  What functions
are declared as __stdcall that interact with this? 

Is there a specific error here?  What should be declared NONSTD?  Is that
really the right thing to do?  (Reverting or making static is not right.)

> Well it wont work on win32.

Without that patch, it doesn't work on any non-Win32 platforms.  I think my
recent reorg of files somehow brought a latent build bug up.  -- justin

Re: svn commit: r106843 - /httpd/httpd/trunk/include/ap_listen.h /httpd/httpd/trunk/server/listen.c

Posted by Mladen Turk <mt...@apache.org>.
Justin Erenkrantz wrote:
> --On Monday, November 29, 2004 10:48 AM +0100 Mladen Turk 
> <mt...@apache.org> wrote:
> 
>> Your patch breaks win32 build.
>> The reason is that the AP_DECLARE is __stdcall while
>> cmd_table is __cdecl.
> 
> 
> Huh?  I don't know Win32 at all, so this makes no sense.
> 

__stdcall: The callee cleans the stack
__cdecl (default): The stack is cleaned up by the caller

So you can not mix that two.
The cmd_table uses __cdecl functions and you are providing
__stdcall function, so the build fails.

>> Can you revert the patch or use the AP_DECLARE_NONSTD
>> or simple extern?
> 
> 
> It has to be AP_DECLARE to be buildable on Unix as the ap_listen.h 
> functions need to be in the exports.c hack - otherwise things don't 
> work.  -- justin
>

Well it wont work on win32.


MT.

Re: svn commit: r106843 - /httpd/httpd/trunk/include/ap_listen.h /httpd/httpd/trunk/server/listen.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 01:03 PM 11/29/2004, Mladen Turk wrote:
>William A. Rowe, Jr. wrote:
>>The reason this popped up is that the function was not
>>previously exported, therefore it was in the c convention
>>as a local function.
>>In the future, Mladen, please don't assume unix coders know
>>this distinction and please go ahead, fix things up as necessary.
>
>Have no idea how to do that with AP_DECLARE.

You don't, simply s/AP_DECLARE/AP_DECLARE_NONSTD with a small 
footnote to the effect that "This fn is registered as a callback
in the cmd_table, and must be declared NONSTD."  Simply fix it,
no hassle, it's already our convention throughout the code for
registered callbacks.

AFAIK - AP_DECLARE, AP_DECLARE_NONSTD and AP_DECLARE_DATA are
all picked up by the exports.c generator.

Bill 


Re: svn commit: r106843 - /httpd/httpd/trunk/include/ap_listen.h /httpd/httpd/trunk/server/listen.c

Posted by Mladen Turk <mt...@apache.org>.
William A. Rowe, Jr. wrote:
> 
> The reason this popped up is that the function was not
> previously exported, therefore it was in the c convention
> as a local function.
> 
> In the future, Mladen, please don't assume unix coders know
> this distinction and please go ahead, fix things up as necessary.

Have no idea how to do that with AP_DECLARE.
So either declarations inside ap_listen.h will change to something
that is acceptable on all platforms, or we'll be unable to build
on win32 anymore AFAICT.

Only thing that I can think of is to add new proto
AP_DECLARE_STD that will be declared as:
#define AP_DECLARE_STD(type)            type

and adding that to make_exports.awk
/^[ \t]*AP[RU]?_(CORE_)?DECLARE?(_STD)[^(]*[(][^)]*[)]([^ ]* )*[^(]+[(]/ {
     sub("[ \t]*AP[RU]?_(CORE_)?DECLARE?(_STD)[^(]*[(][^)]*[)][ \t]*", "")



> Platform maintainers accept that folks will break code on their 
> platform - it's a given.
> 

I'm aware of that. Just fixed couple of things related to mod_cache
and pcre.



MT.

Re: svn commit: r106843 - /httpd/httpd/trunk/include/ap_listen.h /httpd/httpd/trunk/server/listen.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 10:45 AM 11/29/2004, Justin Erenkrantz wrote:
>--On Monday, November 29, 2004 10:48 AM +0100 Mladen Turk <mt...@apache.org> wrote:
>
>>Your patch breaks win32 build.
>>The reason is that the AP_DECLARE is __stdcall while
>>cmd_table is __cdecl.
>
>Huh?  I don't know Win32 at all, so this makes no sense.
>
>>Can you revert the patch or use the AP_DECLARE_NONSTD
>>or simple extern?
>
>It has to be AP_DECLARE to be buildable on Unix as the ap_listen.h functions need to be in the exports.c hack - otherwise things don't work.  -- justin

Unix should gather all AP_DECLARE, AP_DECLARE_NONSTD 
and AP_DECLARE_DATA sections.

The difference for Win32 is that AP_DECLARE uses the 'standard'
calling convention instead of the c flavor.  This usually works
and even speeds things up a bit, until we hit a function that
takes variable arg lists.  That's when things break down, the
callee (function) can't pop the stack on return, because it can
never be quite sure how much arg data the caller pushed onto
the stack.  That's when we use AP_DECLARE_NONSTD.

In this case, registered hook functions are declared nonstd 
(the c flavor) because we aren't quite sure that users won't
abuse one function in favor of another (e.g. have an extra
arguement or two that the hooked function is unaware of.)
Obviously it's also possible for a hook function to take
var args and we didn't want to make a dozen oddball cases
to cover these.

The reason this popped up is that the function was not
previously exported, therefore it was in the c convention
as a local function.

In the future, Mladen, please don't assume unix coders know
this distinction and please go ahead, fix things up as necessary.
Platform maintainers accept that folks will break code on their 
platform - it's a given.

Bill




Re: svn commit: r106843 - /httpd/httpd/trunk/include/ap_listen.h /httpd/httpd/trunk/server/listen.c

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Monday, November 29, 2004 10:48 AM +0100 Mladen Turk <mt...@apache.org> 
wrote:

> Your patch breaks win32 build.
> The reason is that the AP_DECLARE is __stdcall while
> cmd_table is __cdecl.

Huh?  I don't know Win32 at all, so this makes no sense.

> Can you revert the patch or use the AP_DECLARE_NONSTD
> or simple extern?

It has to be AP_DECLARE to be buildable on Unix as the ap_listen.h functions 
need to be in the exports.c hack - otherwise things don't work.  -- justin

Re: svn commit: r106843 - /httpd/httpd/trunk/include/ap_listen.h /httpd/httpd/trunk/server/listen.c

Posted by Mladen Turk <mt...@apache.org>.
jerenkrantz@apache.org wrote:
> Log:
> Properly export ap_listen_* functions.
> 
> * server/listen.c: Add AP_DECLARE as appropriate.
> * include/ap_listen.h: Add AP_DECLARE as appropriate.
>

Hi,

Your patch breaks win32 build.
The reason is that the AP_DECLARE is __stdcall while
cmd_table is __cdecl.

Can you revert the patch or use the AP_DECLARE_NONSTD
or simple extern?


Regards,
Mladen.