You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by do...@apache.org on 2001/11/07 06:41:22 UTC

cvs commit: httpd-2.0/server protocol.c

dougm       01/11/06 21:41:22

  Modified:    include  http_protocol.h
               server   protocol.c
  Log:
  ap_getline() is useful to modules outside of the core..
  change declaration from AP_CORE_DECLARE to AP_DECLARE so it can be used
  used outside the core
  
  Revision  Changes    Path
  1.66      +1 -1      httpd-2.0/include/http_protocol.h
  
  Index: http_protocol.h
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/include/http_protocol.h,v
  retrieving revision 1.65
  retrieving revision 1.66
  diff -u -r1.65 -r1.66
  --- http_protocol.h	2001/09/28 13:48:43	1.65
  +++ http_protocol.h	2001/11/07 05:41:22	1.66
  @@ -526,7 +526,7 @@
    */
   AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, const char *uri);
   
  -AP_CORE_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold);
  +AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold);
   
   /**
    * Get the method number associated with the given string, assumed to
  
  
  
  1.51      +1 -1      httpd-2.0/server/protocol.c
  
  Index: protocol.c
  ===================================================================
  RCS file: /home/cvs/httpd-2.0/server/protocol.c,v
  retrieving revision 1.50
  retrieving revision 1.51
  diff -u -r1.50 -r1.51
  --- protocol.c	2001/10/31 21:14:23	1.50
  +++ protocol.c	2001/11/07 05:41:22	1.51
  @@ -198,7 +198,7 @@
    *        If no LF is detected on the last line due to a dropped connection 
    *        or a full buffer, that's considered an error.
    */
  -AP_CORE_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold)
  +AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold)
   {
       char *pos = s;
       char *last_char;
  
  
  

Re: cvs commit: httpd-2.0/server protocol.c

Posted by Doug MacEachern <do...@covalent.net>.
On Wed, 7 Nov 2001, William A. Rowe, Jr. wrote:
 
> So today, what Doug changed should be a noop - and CORE is not much more than
> window dressing to tell folks the API is internal and subject to change.  Not
> that there is much difference in the 2.0 tree between core/non-core, or stable
> APIs and fluid APIs :)

hmm, i swear a few weeks ago when i tried to use ap_getline on win32 i
ended up with an unresolved symbol.  so i just assumed changing to
AP_DECLARE would make it available, since there was no trouble
resolving any of the functions in protocol.c that use AP_DECLARE.

but i just tried with AP_CORE_DECLARE and didn't have any problem.



Re: cvs commit: httpd-2.0/server protocol.c

Posted by Brian Havard <br...@kheldar.apana.org.au>.
On Wed, 7 Nov 2001 08:18:55 -0600, William A. Rowe, Jr. wrote:

>From: "Greg Stein" <gs...@lyra.org>
>Sent: Wednesday, November 07, 2001 3:56 AM
>
>
>> Um. That doesn't look right.
>
>It doesn't but it is ;)
>
>> Those macros are based on *linkage*, not usage. If server/protocol.c occurs
>> within the "core" DLL on Windows, then it must use the AP_CORE_DECLARE()
>> macro.
>> 
>> Those macros have nothing to do with visibility, and everything to do with
>> linkage.
>> 
>> OtherBill knows the most here, so let's hear his opinion here. I only
>> /think/ something may be wrong here.
>
>Would be wrong, except that AP_DECLARE refers to the core module.  The difference
>is an abstract concept that goes back to early Apache 1.3 porting.  In theory, we
>could ignore all AP_CORE_DECLARES (leave them un-dllexported) and have very
>private core module.  In fact, everyone seems to dip their hands in there so
>they are exported, I believe.
>
>So today, what Doug changed should be a noop - and CORE is not much more than
>window dressing to tell folks the API is internal and subject to change.  Not
>that there is much difference in the 2.0 tree between core/non-core, or stable
>APIs and fluid APIs :)

Actually, build/make_exports.awk does NOT currently include
AP_CORE_DECLAREd functions so platforms that depend on it to generate the
list of core exports do not export core functions. This is actually a
problem because mod_proxy uses several core functions so it fails to build
correctly as a DSO. The offending symbols are:

ap_set_config_vectors()
ap_merge_per_dir_configs()
ap_create_request_config()
ap_create_per_dir_config()

Should they get their CORE removed too?

-- 
 ______________________________________________________________________________
 |  Brian Havard                 |  "He is not the messiah!                   |
 |  brianh@kheldar.apana.org.au  |  He's a very naughty boy!" - Life of Brian |
 ------------------------------------------------------------------------------


Re: cvs commit: httpd-2.0/server protocol.c

Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Greg Stein" <gs...@lyra.org>
Sent: Wednesday, November 07, 2001 3:56 AM


> Um. That doesn't look right.

It doesn't but it is ;)

> Those macros are based on *linkage*, not usage. If server/protocol.c occurs
> within the "core" DLL on Windows, then it must use the AP_CORE_DECLARE()
> macro.
> 
> Those macros have nothing to do with visibility, and everything to do with
> linkage.
> 
> OtherBill knows the most here, so let's hear his opinion here. I only
> /think/ something may be wrong here.

Would be wrong, except that AP_DECLARE refers to the core module.  The difference
is an abstract concept that goes back to early Apache 1.3 porting.  In theory, we
could ignore all AP_CORE_DECLARES (leave them un-dllexported) and have very
private core module.  In fact, everyone seems to dip their hands in there so
they are exported, I believe.

So today, what Doug changed should be a noop - and CORE is not much more than
window dressing to tell folks the API is internal and subject to change.  Not
that there is much difference in the 2.0 tree between core/non-core, or stable
APIs and fluid APIs :)

Bill

> > dougm       01/11/06 21:41:22
> > 
> >   Modified:    include  http_protocol.h
> >                server   protocol.c
> >   Log:
> >   ap_getline() is useful to modules outside of the core..
> >   change declaration from AP_CORE_DECLARE to AP_DECLARE so it can be used
> >   used outside the core



Re: cvs commit: httpd-2.0/server protocol.c

Posted by Greg Stein <gs...@lyra.org>.
Um. That doesn't look right.

Those macros are based on *linkage*, not usage. If server/protocol.c occurs
within the "core" DLL on Windows, then it must use the AP_CORE_DECLARE()
macro.

Those macros have nothing to do with visibility, and everything to do with
linkage.

OtherBill knows the most here, so let's hear his opinion here. I only
/think/ something may be wrong here.

Cheers,
-g

On Wed, Nov 07, 2001 at 05:41:22AM -0000, dougm@apache.org wrote:
> dougm       01/11/06 21:41:22
> 
>   Modified:    include  http_protocol.h
>                server   protocol.c
>   Log:
>   ap_getline() is useful to modules outside of the core..
>   change declaration from AP_CORE_DECLARE to AP_DECLARE so it can be used
>   used outside the core
>   
>   Revision  Changes    Path
>   1.66      +1 -1      httpd-2.0/include/http_protocol.h
>   
>   Index: http_protocol.h
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/include/http_protocol.h,v
>   retrieving revision 1.65
>   retrieving revision 1.66
>   diff -u -r1.65 -r1.66
>   --- http_protocol.h	2001/09/28 13:48:43	1.65
>   +++ http_protocol.h	2001/11/07 05:41:22	1.66
>   @@ -526,7 +526,7 @@
>     */
>    AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, const char *uri);
>    
>   -AP_CORE_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold);
>   +AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold);
>    
>    /**
>     * Get the method number associated with the given string, assumed to
>   
>   
>   
>   1.51      +1 -1      httpd-2.0/server/protocol.c
>   
>   Index: protocol.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/protocol.c,v
>   retrieving revision 1.50
>   retrieving revision 1.51
>   diff -u -r1.50 -r1.51
>   --- protocol.c	2001/10/31 21:14:23	1.50
>   +++ protocol.c	2001/11/07 05:41:22	1.51
>   @@ -198,7 +198,7 @@
>     *        If no LF is detected on the last line due to a dropped connection 
>     *        or a full buffer, that's considered an error.
>     */
>   -AP_CORE_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold)
>   +AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold)
>    {
>        char *pos = s;
>        char *last_char;
>   
>   
>   

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

Re: cvs commit: httpd-2.0/server protocol.c

Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Ryan Morgan" <rm...@covalent.net>
Sent: Monday, November 12, 2001 6:07 PM


> That's why I suggested adding an extra length parameter to ap_getline that 
> gets modified to the length of the data read.. or am I missing something?

I missed that ... +1 on concept!

> IMO the current prototype is lacking since we only know if an error occurred,
> and not what the exact error was.  This may be acceptable for HTTP, but 
> other protocols may require more knowledge for reporting back to the client. 

No, you are dead on here.  This api change is definately worthwhile.

Bill


Re: cvs commit: httpd-2.0/server protocol.c

Posted by Ryan Morgan <rm...@covalent.net>.
On Mon, Nov 12, 2001 at 05:43:25PM -0600, William A. Rowe, Jr. wrote:
> From: "Ryan Morgan" <rm...@covalent.net>
> Sent: Monday, November 12, 2001 4:29 PM
> 
> 
> > It would be even better if ap_getline would return an apr_status_t rather
> > than the number of bytes read.  This would allow callers of ap_getline to
> > detect errors like timeouts, etc from within ap_getline.  Currently we just
> > return the number of bytes read, or -1 on error.
> > 
> > The length parameter could be passed into the function and modified like
> > is done in apr calls that return data.
> > 
> > Thoughts anyone?  I could put together a patch if others see this as a
> > useful feature.
> 
> If used properly, the length saves us a strlen() call, so the existing
> prototype is rather useful.
>

That's why I suggested adding an extra length parameter to ap_getline that 
gets modified to the length of the data read.. or am I missing something?

IMO the current prototype is lacking since we only know if an error occurred,
and not what the exact error was.  This may be acceptable for HTTP, but 
other protocols may require more knowledge for reporting back to the client. 

-Ryan 

Re: cvs commit: httpd-2.0/server protocol.c

Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Ryan Morgan" <rm...@covalent.net>
Sent: Monday, November 12, 2001 4:29 PM


> It would be even better if ap_getline would return an apr_status_t rather
> than the number of bytes read.  This would allow callers of ap_getline to
> detect errors like timeouts, etc from within ap_getline.  Currently we just
> return the number of bytes read, or -1 on error.
> 
> The length parameter could be passed into the function and modified like
> is done in apr calls that return data.
> 
> Thoughts anyone?  I could put together a patch if others see this as a
> useful feature.

If used properly, the length saves us a strlen() call, so the existing
prototype is rather useful.



Re: cvs commit: httpd-2.0/server protocol.c

Posted by Ryan Morgan <rm...@covalent.net>.
It would be even better if ap_getline would return an apr_status_t rather
than the number of bytes read.  This would allow callers of ap_getline to
detect errors like timeouts, etc from within ap_getline.  Currently we just
return the number of bytes read, or -1 on error.

The length parameter could be passed into the function and modified like
is done in apr calls that return data.

Thoughts anyone?  I could put together a patch if others see this as a
useful feature.

-Ryan

On Wed, Nov 07, 2001 at 05:41:22AM -0000, dougm@apache.org wrote:
> dougm       01/11/06 21:41:22
> 
>   Modified:    include  http_protocol.h
>                server   protocol.c
>   Log:
>   ap_getline() is useful to modules outside of the core..
>   change declaration from AP_CORE_DECLARE to AP_DECLARE so it can be used
>   used outside the core
>   
>   Revision  Changes    Path
>   1.66      +1 -1      httpd-2.0/include/http_protocol.h
>   
>   Index: http_protocol.h
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/include/http_protocol.h,v
>   retrieving revision 1.65
>   retrieving revision 1.66
>   diff -u -r1.65 -r1.66
>   --- http_protocol.h	2001/09/28 13:48:43	1.65
>   +++ http_protocol.h	2001/11/07 05:41:22	1.66
>   @@ -526,7 +526,7 @@
>     */
>    AP_CORE_DECLARE(void) ap_parse_uri(request_rec *r, const char *uri);
>    
>   -AP_CORE_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold);
>   +AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold);
>    
>    /**
>     * Get the method number associated with the given string, assumed to
>   
>   
>   
>   1.51      +1 -1      httpd-2.0/server/protocol.c
>   
>   Index: protocol.c
>   ===================================================================
>   RCS file: /home/cvs/httpd-2.0/server/protocol.c,v
>   retrieving revision 1.50
>   retrieving revision 1.51
>   diff -u -r1.50 -r1.51
>   --- protocol.c	2001/10/31 21:14:23	1.50
>   +++ protocol.c	2001/11/07 05:41:22	1.51
>   @@ -198,7 +198,7 @@
>     *        If no LF is detected on the last line due to a dropped connection 
>     *        or a full buffer, that's considered an error.
>     */
>   -AP_CORE_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold)
>   +AP_DECLARE(int) ap_getline(char *s, int n, request_rec *r, int fold)
>    {
>        char *pos = s;
>        char *last_char;
>   
>   
>