[patch modules/generators/mod_status.c]
this looks like a copy-n-paste porting bug. in httpd-2.0's scoreboard stop/start timestamps are in apr_time_t (==usecs only). the problem wasn't noticed before since these structures are never set in workers. Any ideas why? See mod_status's output for yourself. Index: modules/generators/mod_status.c === RCS file: /home/cvspublic/httpd-2.0/modules/generators/mod_status.c,v retrieving revision 1.54 diff -u -r1.54 mod_status.c --- modules/generators/mod_status.c 4 Mar 2002 10:46:22 - 1.54 +++ modules/generators/mod_status.c 11 Mar 2002 09:33:44 - @@ -575,8 +575,7 @@ req_time = 0L; else req_time = (long) -(((ws_record.stop_time - ws_record.start_time) * 1000) -+ ((ws_record.stop_time - ws_record.start_time) / 1000)); +((ws_record.stop_time - ws_record.start_time) / 1000); #endif if (req_time 0L) req_time = 0L; _ Stas Bekman JAm_pH -- Just Another mod_perl Hacker http://stason.org/ mod_perl Guide http://perl.apache.org/guide mailto:[EMAIL PROTECTED] http://ticketmaster.com http://apacheweek.com http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
Re: [patch modules/generators/mod_status.c]
On Mon, 11 Mar 2002, Stas Bekman wrote: this looks like a copy-n-paste porting bug. in httpd-2.0's scoreboard stop/start timestamps are in apr_time_t (==usecs only). the problem wasn't noticed before since these structures are never set in workers. Any ideas why? See mod_status's output for yourself. here is one more porting leftover fix, including the previous one: Index: modules/generators/mod_status.c === RCS file: /home/cvspublic/httpd-2.0/modules/generators/mod_status.c,v retrieving revision 1.54 diff -u -r1.54 mod_status.c --- modules/generators/mod_status.c 4 Mar 2002 10:46:22 - 1.54 +++ modules/generators/mod_status.c 11 Mar 2002 09:43:25 - @@ -570,13 +570,11 @@ req_time = 0L; #endif /* HAVE_TIMES */ #else -if (ws_record.start_time == 0L - ws_record.start_time == 0L) +if (ws_record.start_time == 0L) req_time = 0L; else req_time = (long) -(((ws_record.stop_time - ws_record.start_time) * 1000) -+ ((ws_record.stop_time - ws_record.start_time) / 1000)); +((ws_record.stop_time - ws_record.start_time) / 1000); #endif if (req_time 0L) req_time = 0L; _ Stas Bekman JAm_pH -- Just Another mod_perl Hacker http://stason.org/ mod_perl Guide http://perl.apache.org/guide mailto:[EMAIL PROTECTED] http://ticketmaster.com http://apacheweek.com http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
Code questions (server/mpm_common.c)
Hi, server/mpm_common.c:363 #if defined(QNX) || defined(MPE) || defined(BEOS) || defined(_OSD_POSIX) || defined(TPF) || defined(__TANDEM) || defined(OS2) || defined(WIN32) || defined(NETWARE) Can I break this line into smaller chunks? If so, how? I seem a bit rusty on how the various preprocessors handle multiline #ifs (if they even do). server/mpm_common.c:442 rv = apr_file_close(pod-pod_in); if (rv != APR_SUCCESS) { return rv; } return rv; } If we are going to waste the if, we might aswell return APR_SUCCESS, no? Sander
Minor(?) style questions
Hi, Just checking if people have given this some thought before. And, maybe if there was something decided on this matter. For instance, that this is free when it comes to style rules. As some of you have noticed I am doing style reviews of/ corrections on the current httpd codebase, which has the nice sideeffect of getting us some basic code review aswell. With everything in a consistent style, review by other parties will be easier aswell. Not to mention that one can probably read the code quicker... So, just a few simple questions: 1) Can we decide on a standard style when it comes to using ++ or --? Example: lines++; vs. ++lines; We are currently mixing this in the current code base. I personally favor the first. And unless we are testing the variable in an expression, pre- or postfix doesn't matter for the resulting binary, only for readability. 2) How should a do {} while look like? do { ... } while (...); or do { ... } while (...); 3) What is the preferred method of doing an infinite loop? while (1) { ... } or for (;;) { ... } Thanks, Sander
RE: cvs commit: httpd-2.0/docs/manual/developer filters.html index.html
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]] Sent: 11 March 2002 13:04 To: [EMAIL PROTECTED] Subject: cvs commit: httpd-2.0/docs/manual/developer filters.html index.html striker 02/03/11 04:03:44 Modified:docs/manual/developer index.html Added: docs/manual/developer filters.html Log: Add the How filters work section. Cut 'n paste job from Ryans email 022501c1c529$f63a9550$7f0a@KOJ, needs formatting. Submitted by: Ryan Bloom I just dropped it in, because I didn't want to see it get 'lost' in the archives. Sander
Code questions (server/protocol.c)
Hi, server/protocol.c:136 if (ap_strcasestr(type, charset=) != NULL) { /* already has parameter, do nothing */ /* XXX we don't check the validity */ ; } Validity checking seems like a good idea, someone want to grab this one? server/protocol.c:658 #if 0 /* XXX If we want to keep track of the Method, the protocol module should do * it. That support isn't in the scoreboard yet. Hopefully next week * sometime. rbb */ ap_update_connection_status(AP_CHILD_THREAD_FROM_ID(conn-id), Method, r-method); #endif Can this block go? 'next week' has been over 6 months now :) server/protocol.c:823 r-request_config = ap_create_request_config(r-pool); /* Must be set before we run create request hook */ r-proto_output_filters = conn-output_filters; r-output_filters = r-proto_output_filters; r-proto_input_filters = conn-input_filters; r-input_filters = r-proto_input_filters; ap_run_create_request(r); To what code does the comment refer? The line above it, or the block under it? server/protocol.c:1133 /* Humm, is this check the best it can be? * - protocol = HTTP/1.1 implies support for chunking * - non-keepalive implies the end of byte stream will be signaled *by a connection close * In both cases, we can send bytes to the client w/o needing to * compute content-length. * Todo: * We should be able to force connection close from this filter * when we see we are buffering too much. */ The comment says it all. server/protocol.c:1290 AP_DECLARE(size_t) ap_send_mmap(apr_mmap_t *mm, request_rec *r, size_t offset, size_t length) I reckon the size_t's are left here intentional, weren't forgotten when switching to apr_size_t? server/protocol.c:1338 /* future optimization: record some flags in the request_rec to * say whether we've added our filter, and whether it is first. */ Still valid? server/protocol.c:1501 /* ### TODO: if the total output is large, put all the strings * ### into a single brigade, rather than flushing each time we * ### fill the buffer */ And that's another one for our performance freaks ;) Sander
Re: Minor(?) style questions
- Hi, Just checking if people have given this some thought before. And, maybe if there was something decided on this matter. For instance, that this is free when it comes to style rules. As some of you have noticed I am doing style reviews of/ corrections on the current httpd codebase, which has the nice sideeffect of getting us some basic code review aswell. With everything in a consistent style, review by other parties will be easier aswell. Not to mention that one can probably read the code quicker... So, just a few simple questions: 1) Can we decide on a standard style when it comes to using ++ or --? Example: lines++; vs. ++lines; We are currently mixing this in the current code base. I personally favor the first. And unless we are testing the variable in an expression, pre- or postfix doesn't matter for the resulting binary, only for readability. I would vote for both :-) Either are equally readable IMHO (I tend to use the first, FWIW). 2) How should a do {} while look like? do { ... } while (...); or do { ... } while (...); The first style is more compact and not less readable than the second, IMO. 3) What is the preferred method of doing an infinite loop? while (1) { ... } or for (;;) { ... } I like using the while(1) style. No justification ... Thanks, Sander I would prefer to keep the style guidelines as small as possible. I would be in favor of adding 2) to the existing guidelines and leaving 1) and 2) personal choice. Bill
Re: Code questions (server/mpm_common.c)
Can this be turned into a feature macro? Bill Hi, server/mpm_common.c:363 #if defined(QNX) || defined(MPE) || defined(BEOS) || defined(_OSD_POSIX) || defined(TPF) || defined(__TANDEM) || defined(OS2) || defined(WIN32) || defined(NETWARE) Can I break this line into smaller chunks? If so, how? I seem a bit rusty on how the various preprocessors handle multiline #ifs (if they even do). server/mpm_common.c:442 rv = apr_file_close(pod-pod_in); if (rv != APR_SUCCESS) { return rv; } return rv; } If we are going to waste the if, we might aswell return APR_SUCCESS, no? Sander
Re: Minor(?) style questions
Bill Stoddard [EMAIL PROTECTED] writes: - Hi, 2) How should a do {} while look like? do { ... } while (...); or do { ... } while (...); The first style is more compact and not less readable than the second, IMO. 3) What is the preferred method of doing an infinite loop? while (1) { ... } or for (;;) { ... } I like using the while(1) style. No justification ... Thanks, Sander I would prefer to keep the style guidelines as small as possible. +1! I would be in favor of adding 2) to the existing guidelines +0.4 (slightly nicer style IMHO, but such a specification conflicts with desire to keep the style guidelines simple) and leaving 1) and 2) personal choice. if you mean leaving 1) and 3) personal choice: +1 -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: Minor(?) style questions
\ I would be in favor of adding 2) to the existing guidelines +0.4 (slightly nicer style IMHO, but such a specification conflicts with desire to keep the style guidelines simple) and leaving 1) and 2) personal choice. if you mean leaving 1) and 3) personal choice: +1 Yes that is what I ment. Mr. Bimbo who lives in my finger has a different opinion :-) Bill
Re: Code questions (server/mpm_common.c)
Sander Striker [EMAIL PROTECTED] writes: Hi, server/mpm_common.c:363 #if defined(QNX) || defined(MPE) || defined(BEOS) || defined(_OSD_POSIX) || defined(TPF) || defined(__TANDEM) || defined(OS2) || defined(WIN32) || defined(NETWARE) Can I break this line into smaller chunks? If so, how? I seem a bit rusty on how the various preprocessors handle multiline #ifs (if they even do). Look in ap_config.h for the way to do it (look for SUNOS4) Why not move this OS-specific stuff to ap_config.h and define AP_HAVE_SUPLEMENTARY_GROUPS if we aren't on one of those platforms, and then use AP_HAVE_SUPPLEMENTARY_GROUPS to see what to do? (I'm assuming that the presence of setgrent() and friends isn't good enough to make the decision.) server/mpm_common.c:442 rv = apr_file_close(pod-pod_in); if (rv != APR_SUCCESS) { return rv; } return rv; } If we are going to waste the if, we might aswell return APR_SUCCESS, no? plenty of ways to skin a cat, all of them good as for this code, go for the APR_SUCCESS (you're left with a trade-off between compactness vs. using a style that can be extended if more work is added later) -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
RE: Code questions (server/mpm_common.c)
From: [EMAIL PROTECTED] [mailto:[EMAIL PROTECTED]]On Behalf Of Jeff Trawick Sent: 11 March 2002 15:17 Sander Striker [EMAIL PROTECTED] writes: Hi, server/mpm_common.c:363 #if defined(QNX) || defined(MPE) || defined(BEOS) || defined(_OSD_POSIX) || defined(TPF) || defined(__TANDEM) || defined(OS2) || defined(WIN32) || defined(NETWARE) Can I break this line into smaller chunks? If so, how? I seem a bit rusty on how the various preprocessors handle multiline #ifs (if they even do). Look in ap_config.h for the way to do it (look for SUNOS4) Why not move this OS-specific stuff to ap_config.h and define AP_HAVE_SUPLEMENTARY_GROUPS if we aren't on one of those platforms, and then use AP_HAVE_SUPPLEMENTARY_GROUPS to see what to do? (I'm assuming that the presence of setgrent() and friends isn't good enough to make the decision.) Takers? server/mpm_common.c:442 rv = apr_file_close(pod-pod_in); if (rv != APR_SUCCESS) { return rv; } return rv; } If we are going to waste the if, we might aswell return APR_SUCCESS, no? plenty of ways to skin a cat, all of them good As a cat owner I sincerely how this is a figure of speech... :) as for this code, go for the APR_SUCCESS (you're left with a trade-off between compactness vs. using a style that can be extended if more work is added later) Yah. Will do. Just raising this for anyone else coding in this kind of style. Sander
RE: cvs commit: httpd-2.0/docs/manual/developer filters.html index.html
striker 02/03/11 04:03:44 Modified:docs/manual/developer index.html Added: docs/manual/developer filters.html Log: Add the How filters work section. Cut 'n paste job from Ryans email 022501c1c529$f63a9550$7f0a@KOJ, needs formatting. Submitted by: Ryan Bloom I just dropped it in, because I didn't want to see it get 'lost' in the archives. Thanks Sander, I haven't had time to do this recently. Ryan
status of mod_status
The mod_status module seems to have loads of dead code, at least the code for the #if HAVE_TIMES, which should go away. What's the general status of this module and the scoreboard? I see that not all things from 1.x are working/available yet in the new scoreboard. Thanks to Doug for the hint ;) _ Stas Bekman JAm_pH -- Just Another mod_perl Hacker http://stason.org/ mod_perl Guide http://perl.apache.org/guide mailto:[EMAIL PROTECTED] http://ticketmaster.com http://apacheweek.com http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
Re: status of mod_status
Stas Bekman [EMAIL PROTECTED] writes: The mod_status module seems to have loads of dead code, at least the code for the #if HAVE_TIMES, which should go away. What's the general status of this module and the scoreboard? I see that not all things from 1.x are working/available yet in the new scoreboard. The HAVE_TIMES code is live. At least I see that HAVE_TIMES is defined in my ap_config_auto.h server/scoreboard.c fills in some information when HAVE_TIMES is defined mod_status seems to do something with that information -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: Code questions (server/mpm_common.c)
Sander Striker [EMAIL PROTECTED] writes: plenty of ways to skin a cat, all of them good As a cat owner I sincerely how this is a figure of speech... :) if only you knew our cat :) -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: status of mod_status
Jeff Trawick wrote: Stas Bekman [EMAIL PROTECTED] writes: The mod_status module seems to have loads of dead code, at least the code for the #if HAVE_TIMES, which should go away. What's the general status of this module and the scoreboard? I see that not all things from 1.x are working/available yet in the new scoreboard. The HAVE_TIMES code is live. At least I see that HAVE_TIMES is defined in my ap_config_auto.h server/scoreboard.c fills in some information when HAVE_TIMES is defined mod_status seems to do something with that information oops, should have mentioned which code I was referring to. The ifdef HAVE_TIMES part of a code snippet seems to be irrelevant (includes my previously posted patch as well): Index: modules/generators/mod_status.c === RCS file: /home/cvspublic/httpd-2.0/modules/generators/mod_status.c,v retrieving revision 1.54 diff -u -r1.54 mod_status.c --- modules/generators/mod_status.c 4 Mar 2002 10:46:22 - 1.54 +++ modules/generators/mod_status.c 11 Mar 2002 17:04:43 - @@ -558,26 +558,12 @@ ws_record = ap_scoreboard_image-servers[i][j]; ps_record = ap_scoreboard_image-parent[i]; -#if defined(NO_GETTIMEOFDAY) -#ifdef HAVE_TIMES -if (ws_record.start_time == (clock_t)0) -req_time = 0L; -else { -req_time = ws_record.stop_time - ws_record.start_time; -req_time = (req_time * 1000) / (int)tick; -} -#else -req_time = 0L; -#endif /* HAVE_TIMES */ -#else -if (ws_record.start_time == 0L - ws_record.start_time == 0L) +if (ws_record.start_time == 0L) req_time = 0L; else req_time = (long) -(((ws_record.stop_time - ws_record.start_time) * 1000) -+ ((ws_record.stop_time - ws_record.start_time) / 1000)); -#endif +((ws_record.stop_time - ws_record.start_time) / 1000); + if (req_time 0L) req_time = 0L; -- _ Stas Bekman JAm_pH -- Just Another mod_perl Hacker http://stason.org/ mod_perl Guide http://perl.apache.org/guide mailto:[EMAIL PROTECTED] http://ticketmaster.com http://apacheweek.com http://singlesheaven.com http://perl.apache.org http://perlmonth.com/
FW: zlib vulnerability
We should probably do something about this, but I'm not sure what. Ryan -- Ryan Bloom [EMAIL PROTECTED] 645 Howard St. [EMAIL PROTECTED] San Francisco, CA -Original Message- From: GOMEZ Henri [mailto:[EMAIL PROTECTED]] Sent: Monday, March 11, 2002 3:54 PM To: Ryan Bloom Subject: zlib vulnerability Hi Ryan, Sorry to disturb you but a quick note to warn you about a vulnerability in zlib (which may be used in Apache 2.0 code). http://www.gzip.org/zlib/advisory-2002-03-11.txt Regards - Henri Gomez ___[_] EMAIL : [EMAIL PROTECTED](. .) PGP KEY : 697ECEDD...oOOo..(_)..oOOo... PGP Fingerprint : 9DF8 1EA8 ED53 2F39 DC9B 904A 364F 80E6
Re: FW: zlib vulnerability
Should not matter, unless we staticly link in zlib somewhere. Dynamic linked apps using zlib are fine with a zlib upgrade. Victor -- Victor J. Orlikowski | The Wall is Down, But the Threat Remains! == [EMAIL PROTECTED] | [EMAIL PROTECTED] | [EMAIL PROTECTED]
Re: FW: zlib vulnerability
Recommend that people upgrade, but the vulnerability is *VERY* small. This is merely talking about corruption of malloc structures. To map that into an *application* is practically impossible. It highly depends upon the sequence of malloc() calls, sizes, etc. IOW, we do nothing but recommend zlib 1.1.4. As an aid, we could have an autoconf test for the version and issue a warning. But I don't see code changes needed. Cheers, -g On Mon, Mar 11, 2002 at 03:41:13PM -0800, Ryan Bloom wrote: We should probably do something about this, but I'm not sure what. Ryan ... -Original Message- From: GOMEZ Henri [mailto:[EMAIL PROTECTED]] Sent: Monday, March 11, 2002 3:54 PM To: Ryan Bloom Subject: zlib vulnerability Hi Ryan, Sorry to disturb you but a quick note to warn you about a vulnerability in zlib (which may be used in Apache 2.0 code). http://www.gzip.org/zlib/advisory-2002-03-11.txt Regards - Henri Gomez ___[_] EMAIL : [EMAIL PROTECTED](. .) PGP KEY : 697ECEDD...oOOo..(_)..oOOo... PGP Fingerprint : 9DF8 1EA8 ED53 2F39 DC9B 904A 364F 80E6 -- Greg Stein, http://www.lyra.org/
Re: Minor(?) style questions
On Mon, Mar 11, 2002 at 11:55:07AM +0100, Sander Striker wrote: Hi, Just checking if people have given this some thought before. And, maybe if there was something decided on this matter. For instance, that this is free when it comes to style rules. I think it is fine to make these consistent during your changes, but that style rules on much of this isn't too important. ... 1) Can we decide on a standard style when it comes to using ++ or --? Example: lines++; vs. ++lines; I prefer the latter. The first thing your eye sees is the increment, then the variable. The *operation* is first, which is the most important. ... 2) How should a do {} while look like? do { ... } while (...); or do { ... } while (...); If this isn't already in the style guide, then it should be, as the first form. The control structure is matched with the block. 3) What is the preferred method of doing an infinite loop? while (1) { ... } or for (;;) { ... } I prefer the former. The second form uses implicit rules about 'for', across each of its three control components. Thus, you have to recognize the idiom to quickly understand it, or take a small pause to process it. The while form is also an idiom, but even simpler in construction. As with Bill and Jeff, I'd put #2 into the style guide, and leave the others out. Cheers, -g -- Greg Stein, http://www.lyra.org/
Re: Code questions (server/mpm_common.c)
On Mon, Mar 11, 2002 at 03:33:55PM +0100, Sander Striker wrote: ... server/mpm_common.c:442 rv = apr_file_close(pod-pod_in); if (rv != APR_SUCCESS) { return rv; } return rv; } If we are going to waste the if, we might aswell return APR_SUCCESS, no? Um. Way simpler than that: return apr_file_close(pod-pod_in); Forget the variable, forget the if, etc etc. ... as for this code, go for the APR_SUCCESS (you're left with a trade-off between compactness vs. using a style that can be extended if more work is added later) Um. You're talking about a single line. Go for compact for readibility. Should the day ever arrive to extend, then it is a simple manipulation. Cheers, -g -- Greg Stein, http://www.lyra.org/
Re: FW: zlib vulnerability
Ryan Bloom [EMAIL PROTECTED] writes: We should probably do something about this, but I'm not sure what. I thought the zlib vulnerability was in the decompress path. mod_deflate doesn't decompress. -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
bugfix: 2.0.x Apache Win32 Listeners?
Ok, I'm stumped. The bug is as follows, Apache 2.0 on Win32 lets us use Listen(s) that map to running web servers, such as IIS or even another instance of Apache. This is clearly boneheaded, but that's Win32. First a little bit of background. To actually run apache -k install or -k start/restart, we always test the config. That means we run the server up to post_config, and then perform the action specified with the -k option. -k stop and -k uninstall take things in stride, they too run the same hooks, but pretty much deal with it at the pre_config or rewrite_args phase [don't remember which off hand, just contrasting here.] Now the core fix is trivial, setsockopt for SO_REUSEADDR only after we have called bind. This is safe, since the -child- process isn't running this code fragment, only our parent process. The child inherits those listeners through an mpm_winnt specific code path. And we might be able to to away with SO_REUSEADDR on Win32, but I'm not certain of that. But here's the rub; 1. Command line apache.exe -k start/restart creates/tests listeners 2. Command line apache.exe starts the 'service' process 3. Service apache.exe parent creates listeners - and fails. We have a few options to fix this, such as; 1. never try the listeners by testing if 'service' command options were given (start or restart) and perhaps even skip the listener setup when we install, since that prevents the user from even getting an Apache 2.0.x service to install using the .msi installer, if they already were running a server on the port. 2. move the listeners into pre_mpm (losing the 'listen is valid' test prior to installing the service, and losing the echo-to-console when the listeners are in-use.) 3. close the listeners in post-config if we are about to perform any 'service' -k command such as 'start'... but this still won't solve the 'service' -k restart, since the service is running and bound to the ports. So in reality, this needs to be a combination of the first options for the 'restart' case plus this third option for 'start'. That last oddity really bugs me. There is no way to test the Admin's reconfig of Listen directives before we restart. All other aspects of the server are tested before we let them actually attempt the restart [which generally prevents the admin from hosing their config and then taking down the running server.] So no pretty answers. Comments on the solutions above? I'll commit some fix around noon, so I'd really like some feedback first from our Win32 developer-users. Bill
Re: Minor(?) style questions
On Mon, Mar 11, 2002 at 05:14:47PM -0800, Greg Stein wrote: As with Bill and Jeff, I'd put #2 into the style guide, and leave the others out. 1+. ;-) -- justin
Re: FW: zlib vulnerability
On Mon, Mar 11, 2002 at 08:28:11PM -0500, Jeff Trawick wrote: Ryan Bloom [EMAIL PROTECTED] writes: We should probably do something about this, but I'm not sure what. I thought the zlib vulnerability was in the decompress path. mod_deflate doesn't decompress. Yup. Adler mentioned here on-list that there was a memory leak when using the decompression routines. I'm wondering if that has something to do with this vulnerability. But, yes, I'd say mod_deflate wouldn't be affected unless/until we add input-filtering support. (I think SVN might like this at some point.) -- justin