Re: Squid CygWin enhancement

----- Original Message -----
From: "Guido Serassio" <serassio@libero.it>
To: <squid-dev@squid-cache.org>
Cc: "Robert Collins" <robert.collins@itdomain.com.au>
Sent: Friday, April 27, 2001 2:18 AM
Subject: Re: Squid CygWin enhancement

> Hi Robert,
>
> Il 16.50 26/04/2001 Robert Collins ha scritto:
> >I tried the cygwin branch on a win98 machine today, and it didn't
seem
> >to pickup the DNS from the registry.
> >
> >I'm going to look into that this weekend.
>
> Do You use DHCP or static IP assignment ?
>
>
> >I have a few questions/critiques about the code though (I don't have
> >time to get in and help you just now - so constructive criticism is
all
> >I can offer):
>
>
> Is the welcome ....
>
>
> >1) in defines.h, you are defining _WIN_OS_WIN32S/9X/NT/2K - are these
> >constants tested against the OS? If so should we use the equivalents
> >from windows.h (or files that windows.h auto-includes) ?
>
> These defines are my arbitrary defines, taken from a my code running
on 9x,
> NT and 2000 with
> the GetOSVersion function. There is no useful defines about OS
recognition
> on MS (or CygWin compatible) include files, so i think this code may
be rigth.
>
>
> >2) You've broken parsing /etc/resolv.conf for cygwin. You should
merge
> >in the registry and /etc/resolv.conf, not have only one work.
>
> My idea is that squid compiled with CygWin can be run on a Windows
machine
> without CygWin installed, so for this purpose is better don't use the
> /etc/resolv.conf file. But if You think that the use of
/etc/resolv.conf
> may be useful, is very simple for me re-enable it, better making the
path
> of resolv.conf optionally defined on squid.conf.

If squid if compiled with cygwin support it links to cygwin1.dll. Thus
you need cygwin. I'm suggesting that if you're on cygwin, try to parse
/etc/resolve.conf - but dont require it :]. There's no urgency on this,
I just don't think it should be merged to HEAD without this.

> >3) in tools.c you don't check that remove(to) succeeded. You should
> >check that.
>
> This is true, but this patch is needed only when running on native
mode,
> non under CygWin, and it is not always possible check if remove(to) is
> succesful, because in some situation the 'to' file doesn't exist, for
> example when we make a log file rotate for the first time.

Ok.

> >4) in src/Makefile.in you are always adding win32.o - this should be
> >added conditionally (see how snmp.o is added). (Hint: in configure.in
> >AC_SUBST a value with win32.o if your target in a win32 platform).
>
> Correct, this Makefile.in is a test-only version, my intention was
already
> to ask you for a hint about to do this, so thanks for the hint.
>
> >5) win 32.c you've brought in some winsock2 stuff that shouldn't be
> >brought in just yet IMO. (reasoning: a) the cygwin port: cygwin
cannot
> >combine fork() and direct use of win32 sockets - fork only copies the
> >fd's it knows about. b) for native NT, there is work going on in
another
> >squid branch to allow different socket code to be plugged in
similiarly
> >to the fs modules. The winsock2 code belongs there, not in a top
level
> >file).
>
> the win32.c that you have seen is a brutal cut & paste from the Squid
2.3
> Stable 4. Only a little section of the file that you seen is compiled,
and
> I'm working strong on hit, and now is very different and more clean.

Cool. Thanks.

> >Other than that, great work!
>
> Thanks, I'm happy to announce You that a CygWin squid.exe compiled as
a NT
> service is alive and running without problems.
>

Fantastic.

Rob
Received on Thu Apr 26 2001 - 16:24:46 MDT