httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ben Laurie <...@gonzo.ben.algroup.co.uk>
Subject Re: Comments on current patches.
Date Thu, 14 Sep 1995 08:18:51 GMT
> 
> I just looked at the patches/for_Apache_0.8.13 directory.  A few comments:
> 
> First off, it looks to me like the 01* patches (in for_Apache_0.8.13) are
> intended to fix the problem that the itime patch to mod_log_config which was
> offered last time does not compile.  However, adding the extra include line
> is pointless unless you also toss in the itime modification itself, which is
> not included in either of these patches.  The right thing here is a *single*
> patch (not two, not three) which, when applied to mod_log_config, creates a
> new version which includes the itime patch *and* everything else needed to
> make it compile cleanly in context.  As is, these patches just dink
> mod_log_config without doing anything useful.
> 
> Second, there are a few things I don't understand about the SCO patch.
> 
> *) It increases the size of per-directory configuration vectors by
>    adding the DYNAMIC_MODULE_LIMIT which came in with last week's
>    dynamic modules fix.  I'm not sure that this is necessary at all;
>    per-directory configurations are generally created *after* all
>    dynamically loaded modules are present and initialized.  If it
>    is needed, it should probably be a separate patch.

Umm. My patch doesn't do that, all it does is add a (void **) to the pcalloc().
The DYNAMIC_MODULE_LIMIT bit is in the original, too. Whether it is right
is another question.

> 
> *) It narrows the 'port' element of the server_rec structure from
>    int to short.  Is this the right thing to do?

Ports are short. If I don't narrow it, I get warnings. It might be more
correct to make it an unsigned short. Excerpt from sys/netinet/in.h:

struct sockaddr_in {
        short           sin_family;
        u_short         sin_port;  <--- unsigned short, see?
        struct in_addr  sin_addr;
        char            sin_zero[8];
};

> 
> *) Finally, wrt the change to the Configure script, if you're going
>    to delete the -s option to egrep, the egrep should be redirected into
>    /dev/null; otherwise, when there are syntax errors, we'll get
>    junk output in addition to the error message.

I have to delete -s coz SCO doesn't support it. But, as I read it (from BSDI's
man pages), -s doesn't suppress egrep's output, it suppresses errors caused
by nonexistent or unreadable files. Since egrep is reading from a pipe, -s
is a "no effect" option. Besides, the "junk" output would be the syntax errors.
Rather useful, and not junk at all, IMHO.

> 
> It would be nice if this could all get sorted out before the vote.

Sigh. OK, at some point in the nearish, I'll upload 04c..., but not until
people seem to agree on what it should contain.

Cheers,

Ben.

-- 
Ben Laurie                  Phone: +44 (181) 994 6435
Freelance Consultant        Fax:   +44 (181) 994 6472
and Technical Director      Email: ben@algroup.co.uk (preferred)
A.L. Digital Ltd,                  benl@fear.demon.co.uk (backup)
London, England.

[Note for the paranoid: "fear" as in "Fear and Loathing
in Las Vegas", "demon" as in Demon Internet Services, a
commercial Internet access provider.]

Mime
View raw message