This is the mail archive of the
cygwin-patches@cygwin.com
mailing list for the Cygwin project.
Re: [PATCH] cinstall - "setup grabbing memory by the bucketfull"problem found
Ok, here it is to be reviewed
2001-11-08 Pavel Tsekov <ptsekov@syntrex.com>
* simpsock.h (SimpleSocket::invalidate): Declare new method.
* simpsock.cc (SimpleSocket::invalidate): Implement new method.
(SimpleSocket::SimpleSocket): Initialize buf to zero. Do not allocate
memory for buf in the constructor.
(SimpleSocket::~SimpleSocket): Use SimpleSocket::invalidate().
(SimpleSocket::printf): Use SimpleSocket::write() instead of send().
(SimpleSocket::write): Check object consistency - return -1 on error.
invalidate() the object on socket write error.
(SimpleSocket::fill): Check object consistency - return -1 on error.
invalidate() the object if socket read error is encountered and there
is no more data available in the internal read buffer.
Allocate memory for the internal read buffer.
(SimpleSocket::gets): Return zero (NULL pointer) if error is encountered
during fill() and no more data is available in the internal read buffer.
(SimpleSocket::read): Check object consistency - return -1 on error.
invalidate() the object if socket read error is encountered.
* nio-ftp.cc (NetIO_FTP:NetIO_FTP): Allow 125 as valid response code to
the RETR command (fix for MS IIS ftp server 5 - possibly others too).
(NetIO_FTP::ok): Check if the SimpleSocket object is ok().
(NetIO_FTP::read): Use NetIO_FTP::ok().
* nio-http.cc: Check for valid return value of SimpleSocket::gets().
(NetIO_HTTP::ok): Check if the SimpleSocket object is ok().
Robert Collins wrote:
>
> On Wed, 2001-11-07 at 20:42, Pavel Tsekov wrote:
>
> > Ok now about the patch - I'm just attaching it for review if someone
> > decides
> > to apply it I'll provide a Changelog. What I think is that this is not
> > the best
> > solution to the problem (though it works just fine) - a better one maybe
> > will be
> > some more checking in the SimpleSocket class.
>
> Agreed - more checking in SimpleSocket.
>
> > However I understand that
> > the name
> > Simple implies simplicity :) and as such the designers of this class
>
> >From what I could see, SimpleSocket does perform error checking...
>
> DJ, ohhhh DJ, what was the intent in SimpleSocket?
>
> > decided to
> > leave the error checking for the caller, not the object itself. So my
> > patch
> > complies with this and just check if the object is valid after its
> > creation.
>
> Please submit a patch for this, with more checking in simplesocket, and
> the ftp change for MS borken servers...
diff -up ../cinstall/nio-ftp.cc ./nio-ftp.cc
--- ../cinstall/nio-ftp.cc Thu Nov 8 15:13:16 2001
+++ ./nio-ftp.cc Thu Nov 8 15:12:56 2001
@@ -43,7 +43,7 @@ ftp_line (SimpleSocket *s)
{
do {
last_line = s->gets ();
- log (LOG_BABBLE, "ftp > %s", last_line);
+ log (LOG_BABBLE, "ftp > %s", last_line ? last_line : "error");
} while (last_line && (!isdigit (last_line[0]) || last_line[3] != ' '));
return atoi (last_line ?: "0");
}
@@ -127,7 +127,7 @@ auth_retry:
cmd->printf ("RETR %s\r\n", path);
code = ftp_line (cmd);
- if (code != 150)
+ if (code != 150 && code != 125)
{
delete s;
s = 0;
@@ -144,7 +144,7 @@ NetIO_FTP::~NetIO_FTP ()
int
NetIO_FTP::ok ()
{
- if (s)
+ if (s && s->ok ())
return 1;
return 0;
}
@@ -152,7 +152,7 @@ NetIO_FTP::ok ()
int
NetIO_FTP::read (char *buf, int nbytes)
{
- if (!s)
+ if (!ok ())
return 0;
return s->read (buf, nbytes);
}
diff -up ../cinstall/nio-http.cc ./nio-http.cc
--- ../cinstall/nio-http.cc Thu Nov 8 09:55:01 2001
+++ ./nio-http.cc Thu Nov 8 15:05:04 2001
@@ -121,11 +121,12 @@ NetIO_HTTP::NetIO_HTTP (char *Purl, BOOL
char *l = s->gets ();
int code;
+ if (!l)
+ return;
sscanf (l, "%*s %d", &code);
if (code >= 300 && code < 400)
{
- do {
- l = s->gets ();
+ while ((l = s->gets ()) != 0) {
if (_strnicmp (l, "Location:", 9) == 0)
{
char *u = l + 9;
@@ -135,7 +136,7 @@ NetIO_HTTP::NetIO_HTTP (char *Purl, BOOL
delete s;
goto retry_get;
}
- } while (*l);
+ }
}
if (code == 401) /* authorization required */
{
@@ -169,11 +170,10 @@ NetIO_HTTP::NetIO_HTTP (char *Purl, BOOL
s = 0;
return;
}
- do {
- l = s->gets ();
+ while ((l = s->gets ()) != 0) {
if (_strnicmp (l, "Content-Length:", 15) == 0)
sscanf (l, "%*s %d", &file_size);
- } while (*l);
+ }
}
NetIO_HTTP::~NetIO_HTTP ()
@@ -185,7 +185,7 @@ NetIO_HTTP::~NetIO_HTTP ()
int
NetIO_HTTP::ok ()
{
- if (s)
+ if (s && s->ok ())
return 1;
return 0;
}
Only in .: setup.net.diff
diff -up ../cinstall/simpsock.cc ./simpsock.cc
--- ../cinstall/simpsock.cc Wed Aug 30 03:07:51 2000
+++ ./simpsock.cc Thu Nov 8 14:32:06 2001
@@ -39,7 +39,7 @@ SimpleSocket::SimpleSocket (char *hostna
}
s = INVALID_SOCKET;
- buf = (char *) malloc (SSBUFSZ + 3);
+ buf = 0;
putp = getp = 0;
int i1, i2, i3, i4;
@@ -91,12 +91,7 @@ SimpleSocket::SimpleSocket (char *hostna
SimpleSocket::~SimpleSocket ()
{
- if (s != INVALID_SOCKET)
- closesocket (s);
- s = INVALID_SOCKET;
- if (buf)
- free (buf);
- buf = 0;
+ invalidate ();
}
int
@@ -114,18 +109,28 @@ SimpleSocket::printf (char *fmt, ...)
va_list args;
va_start (args, fmt);
vsprintf (buf, fmt, args);
- return send (s, buf, strlen (buf), 0);
+ return write (buf, strlen (buf));
}
int
SimpleSocket::write (char *buf, int len)
{
- return send (s, buf, len, 0);
+ int rv;
+ if (!ok ())
+ return -1;
+ if ((rv = send (s, buf, len, 0)) == -1)
+ invalidate ();
+ return rv;
}
int
SimpleSocket::fill ()
{
+ if (!ok ())
+ return -1;
+
+ if (buf == 0)
+ buf = (char *) malloc (SSBUFSZ + 3);
if (putp == getp)
putp = getp = 0;
@@ -136,9 +141,12 @@ SimpleSocket::fill ()
if (r > 0)
{
putp += r;
- return r;
}
- return 0;
+ else if (r < 0 && putp == getp)
+ {
+ invalidate();
+ }
+ return r;
}
char *
@@ -151,7 +159,8 @@ SimpleSocket::gets ()
getp = 0;
}
if (putp == getp)
- fill();
+ if (fill () <= 0)
+ return 0;
// getp is zero, always, here, and putp is the count
char *nl;
@@ -165,12 +174,14 @@ SimpleSocket::gets ()
while ((*nl == '\n' || *nl == '\r') && nl >= buf)
*nl-- = 0;
}
- else
+ else if (putp > getp)
{
getp = putp;
nl = buf + putp;
nl[1] = 0;
}
+ else
+ return 0;
return buf;
}
@@ -180,6 +191,9 @@ SimpleSocket::gets ()
int
SimpleSocket::read (char *ubuf, int ulen)
{
+ if (!ok ())
+ return -1;
+
int n, rv=0;
if (putp > getp)
{
@@ -193,11 +207,25 @@ SimpleSocket::read (char *ubuf, int ulen
while (ulen > 0)
{
n = recv (s, ubuf, ulen, 0);
+ if (n < 0)
+ invalidate();
if (n <= 0)
- return rv;
+ return rv > 0 ? rv : n;
ubuf += n;
ulen -= n;
rv += n;
}
return rv;
+}
+
+void
+SimpleSocket::invalidate (void)
+{
+ if (s != INVALID_SOCKET)
+ closesocket (s);
+ s = INVALID_SOCKET;
+ if (buf)
+ free (buf);
+ buf = 0;
+ getp = putp = 0;
}
diff -up ../cinstall/simpsock.h ./simpsock.h
--- ../cinstall/simpsock.h Wed Aug 30 03:05:42 2000
+++ ./simpsock.h Thu Nov 8 13:45:50 2001
@@ -21,6 +21,7 @@ class SimpleSocket {
char *buf;
int putp, getp;
int fill ();
+ void invalidate (void);
public:
SimpleSocket (char *hostname, int port);
Common subdirectories: ../cinstall/temp and ./temp
Common subdirectories: ../cinstall/zlib and ./zlib