You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jim Jagielski <ji...@jaguNET.com> on 1998/05/01 21:50:50 UTC
Adding the OS type
This was raised, and then dropped... Last I recall Ken and Dean had
different views on how to do it, but as far as I can recall, Ken's
had less impact on the API.
Are we just gonna hold off until 1.3.0 is out?
--
===========================================================================
Jim Jagielski ||| jim@jaguNET.com ||| http://www.jaguNET.com/
"That's no ordinary rabbit... that's the most foul,
cruel and bad-tempered rodent you ever laid eyes on"
Re: Adding the OS type
Posted by Dean Gaudet <dg...@arctic.org>.
You can't have less impact on the API when you're trying to extend the API
to support dynamically loaded modules that change the server string :)
You gotta change the API somewhere...
Dean
On Fri, 1 May 1998, Jim Jagielski wrote:
> This was raised, and then dropped... Last I recall Ken and Dean had
> different views on how to do it, but as far as I can recall, Ken's
> had less impact on the API.
>
> Are we just gonna hold off until 1.3.0 is out?
> --
> ===========================================================================
> Jim Jagielski ||| jim@jaguNET.com ||| http://www.jaguNET.com/
> "That's no ordinary rabbit... that's the most foul,
> cruel and bad-tempered rodent you ever laid eyes on"
>
Re: Adding the OS type
Posted by Brian Behlendorf <br...@hyperreal.org>.
At 10:24 PM 5/4/98 -0700, Dean Gaudet wrote:
>
>
>On Mon, 4 May 1998, Brian Behlendorf wrote:
>
>> Teaching webmasters that they *can* edit source code, particularly a
>> harmless #define somewhere, is probably not a bad thing. Recommending they
>> modify that string by using a "AddToVersionString my_pet_hamster/1.2" is
>> fine by me too.
>
>Yeah that works for me too. In fact, don't even bother with the (UNIX),
>(WIN32), (OS2) stuff... just put an AddToVersionString "(UNIX)" into the
>config files.
Except that everyone simply updating their server install rather than
installing fresh will probably not add that... making the numbers far less
statistically valid. And yes, I'm an evil person who cares about usage
patterns :)
Brian
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
pure chewing satisfaction brian@apache.org
brian@hyperreal.org
Re: Adding the OS type
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Dean Gaudet wrote:
>
> If nothing calls ap_add_version_component except the core, this puts an
> extra space at the end of the version string.
Yes, I know - the first patch was, as I said, a 'proof of concept.'
I've fixed this issue in the current work (not yet posted).
> Ah I see. I suppose I added the ap_server_version global because I figured
> you'd flip if I tried to remove ap_get_server_version from buildmark.c,
> which I see is how you solved that particular problem :)
Yup. See, I *can* be convinced that I'm not right. :-)
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: Adding the OS type
Posted by Dean Gaudet <dg...@arctic.org>.
On Tue, 5 May 1998, Rodent of Unusual Size wrote:
> +API_EXPORT(void) ap_add_version_component(const char *component)
> +{
> + if (! version_locked) {
> + /*
> + * If the version string is null, register our cleanup to reset the
> + * pointer on pool destruction.
> + */
> + if (server_version == NULL) {
> + ap_register_cleanup(pconf, NULL, (void (*)(void *))reset_version,
> + NULL);
> + }
> + /* Prepend the given component identifier to the string */
> + server_version = ap_pstrcat(pconf, component, " ", server_version,
> + NULL);
> + }
> +}
If nothing calls ap_add_version_component except the core, this puts an
extra space at the end of the version string.
This is one of those things I meant by "my version was tested".
On Tue, 5 May 1998, Rodent of Unusual Size wrote:
> > I'm curious. Why don't you just start from my patch and remove the pieces
> > which removed SERVER_SUBVERSION? The rest of my patch was completely
> > tested.
>
> In part because you used loaded terms like "ap_add_insecurity," partly
> because you introduced additional global symbols, partly because it
> exposed the version-string cell, partly because it changed the entire
> SERVER_*VERSION handling, and partly because I felt it was a challenge
> to do it in a simpler (MHO) way. After all, *you* didn't start from
> *my* patch, which was also completely tested.. :-)
Ah I see. I suppose I added the ap_server_version global because I figured
you'd flip if I tried to remove ap_get_server_version from buildmark.c,
which I see is how you solved that particular problem :)
Dean
Re: Adding the OS type
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Dean Gaudet wrote:
>
> So !version_locked is false until the next reset after ap_set_version(),
> and ap_add_version_component does nothing. ? Or is it just too early in
> the morning (er, afternoon) for me?
version_locked is false until ap_set_version() is called. It's given
an initial value of false. This guards against calls to ap_..component()
except during the command processing and module init phases, so
the string won't get munged if someone puts a call into one of
the merge functions, for instance. The cleanup resets the value back
to false, opening it up for modification again.
> I'm curious. Why don't you just start from my patch and remove the pieces
> which removed SERVER_SUBVERSION? The rest of my patch was completely
> tested.
In part because you used loaded terms like "ap_add_insecurity," partly
because you introduced additional global symbols, partly because it
exposed the version-string cell, partly because it changed the entire
SERVER_*VERSION handling, and partly because I felt it was a challenge
to do it in a simpler (MHO) way. After all, *you* didn't start from
*my* patch, which was also completely tested.. :-)
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: Adding the OS type
Posted by Dean Gaudet <dg...@arctic.org>.
On Tue, 5 May 1998, Rodent of Unusual Size wrote:
> +API_EXPORT(void) ap_add_version_component(const char *component)
> +{
> + if (! version_locked) {
> + /*
> + * If the version string is null, register our cleanup to reset the
> + * pointer on pool destruction.
> + */
> + if (server_version == NULL) {
> + ap_register_cleanup(pconf, NULL, (void (*)(void *))reset_version,
> + NULL);
> + }
> + /* Prepend the given component identifier to the string */
> + server_version = ap_pstrcat(pconf, component, " ", server_version,
> + NULL);
> + }
> +}
> +
> +/*
> + * This routine adds the real server base identity to the version string,
> + * and then locks out changes until the next reconfig.
> + */
> +static void ap_set_version()
> +{
> + ap_add_version_component(SERVER_BASEVERSION);
> + version_locked++;
> +}
> +
So !version_locked is false until the next reset after ap_set_version(),
and ap_add_version_component does nothing. ? Or is it just too early in
the morning (er, afternoon) for me?
I'm curious. Why don't you just start from my patch and remove the pieces
which removed SERVER_SUBVERSION? The rest of my patch was completely
tested.
Dean
Re: Adding the OS type
Posted by Brian Behlendorf <br...@hyperreal.org>.
At 07:44 AM 5/5/98 -0400, Rodent of Unusual Size wrote:
>All right, here's a partial patch to address this. It's not
>complete because:
>
> o there's no provision for either SERVER_SUBVERSION or a
> AddServerComponentID (or whatever directive)
> o there's no provision for the "(platform)" stuff
>
>Before I add those pieces, I want to make sure people like what I've
>got so far. Preliminary tests indicate that it does the job, but
>I'd appreciate feedback.
+1 so far.
Brian
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
pure chewing satisfaction brian@apache.org
brian@hyperreal.org
Re: Adding the OS type
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
All right, here's a partial patch to address this. It's not
complete because:
o there's no provision for either SERVER_SUBVERSION or a
AddServerComponentID (or whatever directive)
o there's no provision for the "(platform)" stuff
Before I add those pieces, I want to make sure people like what I've
got so far. Preliminary tests indicate that it does the job, but
I'd appreciate feedback. I had to add a LockFile (heretofore
missing from my config) for some reason after adding a test call to
mod_mime, but I think that's unrelated.
Comments?
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Index: buildmark.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/buildmark.c,v
retrieving revision 1.6
diff -u -r1.6 buildmark.c
--- buildmark.c 1998/04/11 12:00:16 1.6
+++ buildmark.c 1998/05/05 11:31:53
@@ -63,14 +63,8 @@
#else
static const char server_built[] = "unknown";
#endif
-static const char server_version[] = SERVER_VERSION;
API_EXPORT(const char *) ap_get_server_built()
{
return server_built;
-}
-
-API_EXPORT(const char *) ap_get_server_version()
-{
- return server_version;
}
Index: include/httpd.h
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/include/httpd.h,v
retrieving revision 1.209
diff -u -r1.209 httpd.h
--- httpd.h 1998/05/04 17:19:54 1.209
+++ httpd.h 1998/05/05 11:31:59
@@ -395,6 +395,7 @@
#endif
API_EXPORT(const char *) ap_get_server_version(void);
+API_EXPORT(void) ap_add_version_component(const char *component);
API_EXPORT(const char *) ap_get_server_built(void);
/* Numeric release version identifier: major minor bugfix betaseq
Index: main/http_main.c
===================================================================
RCS file: /export/home/cvs/apache-1.3/src/main/http_main.c,v
retrieving revision 1.330
diff -u -r1.330 http_main.c
--- http_main.c 1998/05/04 16:46:40 1.330
+++ http_main.c 1998/05/05 11:32:07
@@ -329,13 +329,63 @@
static pool *pchild; /* Pool for httpd child stuff */
static pool *pcommands; /* Pool for -C and -c switches */
-static int APACHE_TLS my_pid; /* it seems silly to call getpid all
the
time */
+static int APACHE_TLS my_pid; /* it seems silly to call getpid all the time
*/
#ifndef MULTITHREAD
static int my_child_num;
#endif
scoreboard *ap_scoreboard_image = NULL;
+/*
+ * Pieces for managing the contents of the Server-Version response header
+ * field.
+ */
+static char *server_version = NULL;
+static int version_locked = 0;
+
+/*
+ * This routine is called when the pconf pool is vacuumed. It resets the
+ * server version string to a known value and [re]enables modifications
+ * (which are disabled by configuration completion).
+ */
+static void reset_version()
+{
+ version_locked = 0;
+ server_version = NULL;
+}
+
+API_EXPORT(const char *) ap_get_server_version()
+{
+ return server_version;
+}
+
+API_EXPORT(void) ap_add_version_component(const char *component)
+{
+ if (! version_locked) {
+ /*
+ * If the version string is null, register our cleanup to reset the
+ * pointer on pool destruction.
+ */
+ if (server_version == NULL) {
+ ap_register_cleanup(pconf, NULL, (void (*)(void *))reset_version,
+ NULL);
+ }
+ /* Prepend the given component identifier to the string */
+ server_version = ap_pstrcat(pconf, component, " ", server_version,
+ NULL);
+ }
+}
+
+/*
+ * This routine adds the real server base identity to the version string,
+ * and then locks out changes until the next reconfig.
+ */
+static void ap_set_version()
+{
+ ap_add_version_component(SERVER_BASEVERSION);
+ version_locked++;
+}
+
static APACHE_TLS int volatile exit_after_unblock = 0;
/* a clean exit from a child with proper cleanup */
@@ -3772,6 +3822,7 @@
ap_open_logs(server_conf, pconf);
ap_log_pid(pconf, ap_pid_fname);
ap_init_modules(pconf, server_conf);
+ ap_set_version();
SAFE_ACCEPT(accept_mutex_init(pconf));
if (!is_graceful) {
reinit_scoreboard(pconf);
@@ -4057,6 +4108,7 @@
if (ap_standalone) {
ap_open_logs(server_conf, pconf);
ap_init_modules(pconf, server_conf);
+ ap_set_version();
STANDALONE_MAIN(argc, argv);
}
else {
@@ -4928,6 +4980,7 @@
server_conf = ap_read_config(pconf, pparent, ap_server_confname);
ap_open_logs(server_conf, pconf);
ap_init_modules(pconf, server_conf);
+ ap_set_version();
if (!is_graceful)
reinit_scoreboard(pconf);
@@ -5163,6 +5216,7 @@
ap_log_pid(pconf, ap_pid_fname);
ap_init_modules(pconf, server_conf);
ap_suexec_enabled = init_suexec();
+ ap_set_version();
ap_open_logs(server_conf, pconf);
set_group_privs();
Re: Adding the OS type
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Brian Behlendorf wrote:
>
> They would still be able to edit SERVER_BASEVERSION in httpd.h.
Not a goodness, IMHO, because that encourages dilution of the *real*
base server identity, "Apache/mumble".
> >In fact, I'd rather have a directive that allowed such additions to
> >be made at run-time rather than requiring a complete rebuild, for
> >exactly this reason.
>
> Such a directive would not need the -DSERVER_SUBVERSION.
Correct.
> Teaching webmasters that they *can* edit source code, particularly a
> harmless #define somewhere, is probably not a bad thing. Recommending they
> modify that string by using a "AddToVersionString my_pet_hamster/1.2" is
> fine by me too.
Teaching them that they can edit source, yes. Forcing them to do so, no.
I'm halfway through a patch that will make even Dean happy (I think).
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: Adding the OS type
Posted by Manoj Kasichainula <ma...@io.com>.
On Mon, May 04, 1998 at 11:36:55PM -0600, Marc Slemko wrote:
> What I'm not thrilled with is suggestions of having a config file
> directive to let people add things to it. Yes, there are advantages to it
> but I'm just not sure it is worth it.
Well, if a legit reason for doing this ever shows up, I'm sure someone
will write a 3rd-party module with this directive.
--
Manoj Kasichainula - manojk at io dot com - http://www.io.com/~manojk/
"Some people have entirely too much free time on their hands."
- Gene Spafford (spaf)
Re: Adding the OS type
Posted by Marc Slemko <ma...@worldgate.com>.
On Mon, 4 May 1998, Dean Gaudet wrote:
[...]
>
> > Letting modules change it, sure although that can lead to the "every
> > module in the server name" situation.
>
> Agreed.
>
> > Letting someone change a define, fine.
>
> And how do you make shared modules work?
That is the proposed API function implicitly refered to in the "let
modules change it". Should be "add to it" instead of "change it".
Talking about setting a define is just because I support including the
backwards compat method.
What I'm not thrilled with is suggestions of having a config file
directive to let people add things to it. Yes, there are advantages to it
but I'm just not sure it is worth it.
>
> Dean
>
> > Going beyond that, it gets trickier.
> >
> >
>
Re: Adding the OS type
Posted by Dean Gaudet <dg...@arctic.org>.
On Mon, 4 May 1998, Marc Slemko wrote:
> On Mon, 4 May 1998, Brian Behlendorf wrote:
>
> > Teaching webmasters that they *can* edit source code, particularly a
> > harmless #define somewhere, is probably not a bad thing. Recommending they
> > modify that string by using a "AddToVersionString my_pet_hamster/1.2" is
> > fine by me too.
>
> I'm really not happy with the idea of encouraging or making it easy for
> every joe yahoo to make up dumb server strings so they can feel cool.
Agreed.
> Aside from being part of my dog-given right to enforce true righteousness
> on the entire world, it causes annoyances when trying to help smart-assed
> people with "bugs" they report.
Agreed.
> Letting modules change it, sure although that can lead to the "every
> module in the server name" situation.
Agreed.
> Letting someone change a define, fine.
And how do you make shared modules work?
Dean
> Going beyond that, it gets trickier.
>
>
Re: Adding the OS type
Posted by Marc Slemko <ma...@worldgate.com>.
On Mon, 4 May 1998, Brian Behlendorf wrote:
> Teaching webmasters that they *can* edit source code, particularly a
> harmless #define somewhere, is probably not a bad thing. Recommending they
> modify that string by using a "AddToVersionString my_pet_hamster/1.2" is
> fine by me too.
I'm really not happy with the idea of encouraging or making it easy for
every joe yahoo to make up dumb server strings so they can feel cool.
Aside from being part of my dog-given right to enforce true righteousness
on the entire world, it causes annoyances when trying to help smart-assed
people with "bugs" they report.
Letting modules change it, sure although that can lead to the "every
module in the server name" situation.
Letting someone change a define, fine.
Going beyond that, it gets trickier.
Re: Adding the OS type
Posted by Dean Gaudet <dg...@arctic.org>.
On Mon, 4 May 1998, Brian Behlendorf wrote:
> Teaching webmasters that they *can* edit source code, particularly a
> harmless #define somewhere, is probably not a bad thing. Recommending they
> modify that string by using a "AddToVersionString my_pet_hamster/1.2" is
> fine by me too.
Yeah that works for me too. In fact, don't even bother with the (UNIX),
(WIN32), (OS2) stuff... just put an AddToVersionString "(UNIX)" into the
config files.
Dean
Re: Adding the OS type
Posted by Brian Behlendorf <br...@hyperreal.org>.
At 10:05 AM 5/4/98 -0400, you wrote:
>The two are not equivalent. You're making the assumption that the
>only thing that should ever contribute to the server version is
>something coded into the binary (e.g., a module). I disagree; I
>think the Webmaster is entitled to make the decision that some
>external-to-httpd addon or plugin is worthy of mention in the
>server version. What that may be, I don't know - but robbing the
>server owner of this flexibility is something with which I don't
>agree. It's *his* server that he's running, after all.
They would still be able to edit SERVER_BASEVERSION in httpd.h.
>In fact, I'd rather have a directive that allowed such additions to
>be made at run-time rather than requiring a complete rebuild, for
>exactly this reason.
Such a directive would not need the -DSERVER_SUBVERSION.
>> Unless Ken wants to rescind his -1 on Dean's patch, I guess we should.
>
>If someone can convince me counter to my remarks in my first
>paragraph above, I'll take it back. As it stands, though, Dean's
>patch disenfranchises Webmasters unless they're also developers - which
>I don't think is fair at all.
Teaching webmasters that they *can* edit source code, particularly a
harmless #define somewhere, is probably not a bad thing. Recommending they
modify that string by using a "AddToVersionString my_pet_hamster/1.2" is
fine by me too.
>I'll wait until I hear a couple more thoughts on this before I do
>anything else; it's very discouraging to keep coming up with ideas
>and getting told they're wrong and stupid. I have thoughts on a
>clean way to do this, but I don't care to get shot down again..
I understand. I'm just less concerned about supporting something that has
only been in the betas.
Brian
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
pure chewing satisfaction brian@apache.org
brian@hyperreal.org
Re: Adding the OS type
Posted by Rodent of Unusual Size <Ke...@Golux.Com>.
Brian Behlendorf wrote:
>
> Actually, Dean's proposed patch removed the -DSERVER_SUBVERSION
> compile-time option, which I think I agree with, for the same philosophical
> reasons he cited. If you have two eays of doing the exact same thing, it
> can become a nightmare to support. Removing redundant functionality is a
> good thing. Perl is not a counter-example. :)
The two are not equivalent. You're making the assumption that the
only thing that should ever contribute to the server version is
something coded into the binary (e.g., a module). I disagree; I
think the Webmaster is entitled to make the decision that some
external-to-httpd addon or plugin is worthy of mention in the
server version. What that may be, I don't know - but robbing the
server owner of this flexibility is something with which I don't
agree. It's *his* server that he's running, after all.
In fact, I'd rather have a directive that allowed such additions to
be made at run-time rather than requiring a complete rebuild, for
exactly this reason.
> Unless Ken wants to rescind his -1 on Dean's patch, I guess we should.
If someone can convince me counter to my remarks in my first
paragraph above, I'll take it back. As it stands, though, Dean's
patch disenfranchises Webmasters unless they're also developers - which
I don't think is fair at all.
I'll wait until I hear a couple more thoughts on this before I do
anything else; it's very discouraging to keep coming up with ideas
and getting told they're wrong and stupid. I have thoughts on a
clean way to do this, but I don't care to get shot down again..
#ken P-)}
Ken Coar <http://Web.Golux.Com/coar/>
Apache Group member <http://www.apache.org/>
"Apache Server for Dummies" <http://Web.Golux.Com/coar/ASFD/>
Re: Adding the OS type
Posted by Brian Behlendorf <br...@hyperreal.org>.
At 03:50 PM 5/1/98 -0400, Jim Jagielski wrote:
>This was raised, and then dropped...
Well, it flamed out...
>Last I recall Ken and Dean had
>different views on how to do it, but as far as I can recall, Ken's
>had less impact on the API.
Actually, Dean's proposed patch removed the -DSERVER_SUBVERSION
compile-time option, which I think I agree with, for the same philosophical
reasons he cited. If you have two eays of doing the exact same thing, it
can become a nightmare to support. Removing redundant functionality is a
good thing. Perl is not a counter-example. :)
>Are we just gonna hold off until 1.3.0 is out?
Unless Ken wants to rescind his -1 on Dean's patch, I guess we should.
Brian
--=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=--
pure chewing satisfaction brian@apache.org
brian@hyperreal.org