This is the mail archive of the cygwin-patches@cygwin.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

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

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]