[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    [PATCH] socket stuff
From:       Thiago Macieira <thiagom () wanadoo ! fr>
Date:       2002-02-10 20:11:33
[Download RAW message or body]

Yea, it's me again.

I've been doing my late homework and I've found out a couple of unwanted uses 
of sockets in kdelibs and kdebase:

kdelibs/arts/:
Uses gethostbyname() and low-level I/O. Can't fix because doesn't link to 
libkdecore. Implications: no IPv6 and no automatic proxying.

kdelibs/kdeprint/management:
Uses gethostbyname(), gethostbyaddr() and low-level I/O. Also includes 
ugly-hacks regarding network size (assumes everyone is sitting in /24 
networks). Can't fix because it doesn't link to libkdecore.

kdelibs/kio/misc/kpac:
Uses gethostbyname(), but doesn't link to libkdecore (only libkjs). Can't fix 
it for now.

kdebase/kdm/chooser:
Uses gethostbyname() and gethostbyaddr(). I didn't even try to fix because I 
don't know of the implications of XDMCP.

kdebase/kioslave/audiocd:
CDDB inteface used KSocket::initSockaddr(), which will be removed. I've 
modified the code for full KExtendedSocket support. Patch attached and 
tested.

kdebase/kioslave/nfs:
Uses gethostbyname(), but also uses lower-level RPC functions that we don't 
control. I thought it would be better to leave alone. NFS-over-IPv6 isn't 
widespread and proxying of NFS calls might be a bad idea.

kdebase/kioslave/nntp:
Used KSocket::initSockaddr() in class TCPWrapper. That class could be almost 
entirely replaced by KExtendedSocket, avoiding code duplication. But I didn't 
try doing that so close to a release. Instead, I've created a minimal patch 
to enable proxying and all, however, I couldn't test it. Patch is attached.

kdebase/kioslave/smbro:
Same thing as the NFS ioslave: we don't really know how the underlying 
protocol works on IPv6, so we had better not change it for a while.

Also, I'd like to submit for review a patch for KExtendedSocket itself. It 
fixes some bugs regarding the socket notifiers, getch() and putch() 
signedness problems, adds readyAccept signal and overrides bytesAvailable for 
the future. Patch is attached as well.
-- 
  Thiago Macieira - UFOT Registry number: 1001
 thiagom@mail.com
   ICQ UIN: 1967141  PGP: 0x8F2978D5 and 0xEA9037A5 (PGP 2.x)
     Registered Linux user #65028

["nntp.patch" (text/x-diff)]

Index: kioslave/nntp/nntp.cpp
===================================================================
RCS file: /home/kde/kdebase/kioslave/nntp/nntp.cpp,v
retrieving revision 1.12
diff -u -3 -p -r1.12 nntp.cpp
--- kioslave/nntp/nntp.cpp	2001/12/29 17:21:40	1.12
+++ kioslave/nntp/nntp.cpp	2002/02/10 20:06:08
@@ -21,7 +21,8 @@
 #include <qdir.h>
 #include <qregexp.h>
 
-#include <ksock.h>
+#include <kextsock.h>
+#include <ksocks.h>
 #include <kapplication.h>
 #include <kdebug.h>
 #include <klocale.h>
@@ -675,6 +676,19 @@ QString& NNTPProtocol::errorStr(int resp
 
 /***************** class TCPWrapper ******************/
 
+// FIXME!
+// There is a whole lot of duplicated code from here and
+// KExtendedSocket. Someone should clean this up one day.
+//
+// The TCPWrapper class in here could be almost entirely replaced
+// by KExtendedSocket without buffering (we cannot use buffering in
+// ioslaves because there is no Qt event loop). The read timeout 
+// behaviour can be obtained with waitForMore, but the write timeout
+// requires more coding. 
+// As of now, we're using KExtendedSocket to connect and
+// KSocks for the I/O.
+// -thiago
+
 TCPWrapper::TCPWrapper()
 {
   timeOut = DEFAULT_TIME_OUT;
@@ -696,6 +710,7 @@ bool TCPWrapper::connect(const QString &
 
   DBG << "socket connecting to " << host << ":" << port << endl;
 
+#if 0
   ksockaddr_in server_name;
 
   // init socket
@@ -719,6 +734,24 @@ bool TCPWrapper::connect(const QString &
     error(ERR_COULD_NOT_CONNECT, host);
     return false;
   }
+#endif
+
+  KExtendedSocket ks(host, port);
+  if (ks.lookup() < 0)
+    {
+      emit error(ERR_UNKNOWN_HOST, host); // untrue
+      return false;
+    }
+
+  if (ks.connect() < 0)
+    {
+      // code above doesn't use emit, but this is a signal
+      emit error(ERR_COULD_NOT_CONNECT, host);
+      return false;
+    }
+
+  tcpSocket = ks.fd();
+  ks.release();			// we're free of KExtendedSocket
 
   return true;
 
@@ -728,7 +761,7 @@ bool TCPWrapper::disconnect() {
 
   // close socket
   if (tcpSocket != -1) {
-    close(tcpSocket);
+    ::close(tcpSocket);
     tcpSocket = -1;
   }
 
@@ -783,7 +816,7 @@ bool TCPWrapper::readData() {
 
     // read bytes from socket
     do {
-      bytes = ::read(tcpSocket, data_end, (buffer+SOCKET_BUFFER_SIZE)-data_end);
+      bytes = KSocks::self()->read(tcpSocket, data_end, (buffer+SOCKET_BUFFER_SIZE)-data_end);
     } while (bytes<0 && errno==EINTR); // ignore signals
 
     if (bytes <= 0) { // there was an error
@@ -823,7 +856,7 @@ bool TCPWrapper::readyForReading(){
     FD_SET(tcpSocket, &fdsE);
     tv.tv_sec = timeOut;
     tv.tv_usec = 0;
-    ret = select(FD_SETSIZE, &fdsR, NULL, &fdsE, &tv);
+    ret = KSocks::self()->select(FD_SETSIZE, &fdsR, NULL, &fdsE, &tv);
     // DBG << "select (r): " << ret << " errno: " << errno << endl;
   } while ((ret<0) && (errno == EINTR)); // ignore signals
 
@@ -864,7 +897,7 @@ bool TCPWrapper::writeData(const QByteAr
 
      while (byteCount < chars) {
 
-       bytes = ::write(tcpSocket, &data.data()[byteCount], chars-byteCount);
+       bytes = KSocks::self()->write(tcpSocket, &data.data()[byteCount], chars-byteCount);
        if (bytes <= 0) {
          ERR << "error writing to socket" << endl;
          emit error(ERR_COULD_NOT_WRITE,strerror(errno));
@@ -896,7 +929,7 @@ bool TCPWrapper::readyForWriting() {
     FD_SET(tcpSocket, &fdsE);
     tv.tv_sec = timeOut;
     tv.tv_usec = 0;
-    ret = select(FD_SETSIZE, NULL, &fdsW, &fdsE, &tv);
+    ret = KSocks::self()->select(FD_SETSIZE, NULL, &fdsW, &fdsE, &tv);
     // DBG << "select (w): " << ret << " errno: " << errno << endl;
   } while ((ret<0) && (errno == EINTR)); // ignore signals
 

["kextsock.patch" (text/x-diff)]

Index: kextsock.h
===================================================================
RCS file: /home/kde/kdelibs/kdecore/kextsock.h,v
retrieving revision 1.17
diff -u -3 -p -r1.17 kextsock.h
--- kextsock.h	2002/01/26 15:58:57	1.17
+++ kextsock.h	2002/02/10 19:50:09
@@ -441,6 +441,11 @@ public:
    * blocking mode, this function will block until a connection is received.
    * Otherwise, it might return with error. The sock parameter will be
    * initialised with the newly created socket.
+   *
+   * Upon successful acception (i.e., this function returns 0), the newly
+   * created socket will be already connected. The socket will be unbuffered
+   * and readyRead() and readyWrite() signals will be disabled.
+   *
    * Returns 0 on success, -1 on system error (errno set) and -2 if this is
    * not a passiveSocket and -3 if this took too long (time out)
    * @param sock	a pointer to an KExtendedSocket variable
@@ -456,6 +461,18 @@ public:
    *   bind address or that an asynchronous connection attempt is already
    *   in progress.
    * @li -3: connection timed out
+   *
+   * After successful connection (return value 0), the socket will be ready
+   * for I/O operations. Note, however, that not all signals may be enabled
+   * for emission by this socket:
+   * @li @ref readyRead and @ref readyWrite signals will be enabled only if
+   *    @ref enableRead or @ref enableWrite were called. You can still enable
+   *    them by calling those functions, of course.
+   * @li @ref closed will only be sent if we are indeed reading from the input
+   *    stream. That is, if this socket is buffering the input. See @ref setBufferSize
+   *
+   * Note that, in general, functions inherited/overriden from KBufferedIO will only
+   * work on buffered sockets, like bytesAvailable and bytesToWrite.
    */
   virtual int connect();
 
@@ -645,6 +662,16 @@ public:
   virtual int unreadBlock(const char *data, uint len);
 
   /**
+   * Returns the number of available bytes yet to be read via @ref readBlock
+   * and family of functions. Returns -1 on error or -2 if this call is invalid
+   * in the current state.
+   *
+   * Note: as of now, this only works on input-buffered sockets. This will
+   * change in the future
+   */
+  virtual int bytesAvailable() const;
+
+  /**
    * Waits @p msec milliseconds for more data to be available (use 0 to
    * wait forever). The return value is the amount of data available for
    * read in the read buffer.
@@ -713,6 +740,12 @@ signals:
    * @param error	the errno code of the last connection attempt
    */
   void connectionFailed(int error);
+
+  /**
+   * This signal is emitted whenever this socket is ready to accept another
+   * socket. @see accept
+   */
+  void readyAccept();
 
 protected:
   int sockfd;			// file descriptor of the socket
Index: kextsock.cpp
===================================================================
RCS file: /home/kde/kdelibs/kdecore/kextsock.cpp,v
retrieving revision 1.32
diff -u -3 -p -r1.32 kextsock.cpp
--- kextsock.cpp	2002/01/26 15:58:58	1.32
+++ kextsock.cpp	2002/02/10 19:50:10
@@ -838,6 +838,18 @@ bool KExtendedSocket::setBufferSize(int 
 
   setFlags((mode() & ~IO_Raw) | ((d->flags & bufferedSocket) ? 0 : IO_Raw));
 
+  // check we didn't turn something off we shouldn't
+  if (d->emitRead && d->qsnIn == NULL)
+    {
+      d->qsnIn = new QSocketNotifier(sockfd, QSocketNotifier::Read);
+      QObject::connect(d->qsnIn, SIGNAL(activated(int)), this, SLOT(socketActivityRead()));
+    }
+  if (d->emitWrite && d->qsnOut == NULL)
+    {
+      d->qsnOut = new QSocketNotifier(sockfd, QSocketNotifier::Write);
+      QObject::connect(d->qsnOut, SIGNAL(activated(int)), this, SLOT(socketActivityWrite()));
+    }
+
   return true;
 }
 
@@ -1065,12 +1077,18 @@ int KExtendedSocket::listen(int N)
       return -1;
     }
 
-  d->status = listening;
+  d->status = bound;
   setFlags(IO_Sequential | IO_Raw | IO_ReadWrite);
 
   int retval = KSocks::self()->listen(sockfd, N);
   if (retval == -1)
     setError(IO_ListenError, errno);
+  else
+    {
+      d->status = listening;
+      d->qsnIn = new QSocketNotifier(sockfd, QSocketNotifier::Read);
+      QObject::connect(d->qsnIn, SIGNAL(activated(int)), this, SLOT(socketActivityRead()));
+    }
   return retval == -1 ? -1 : 0;
 }
 
@@ -1149,7 +1167,7 @@ int KExtendedSocket::accept(KExtendedSoc
 int KExtendedSocket::connect()
 {
   cleanError();
-  if (d->flags & passiveSocket)
+  if (d->flags & passiveSocket || d->status >= connected)
     return -2;
   if (d->status < lookupDone)
     if (lookup() < 0)
@@ -1315,6 +1333,7 @@ int KExtendedSocket::connect()
 	    }
 
 	  // getting here means it connected
+	  // setBufferSize() takes care of creating the socket notifiers
 	  setBlockingMode(true);
 	  d->status = connected;
 	  setFlags(IO_Sequential | IO_Raw | IO_ReadWrite | IO_Open | IO_Async);
@@ -1625,6 +1644,8 @@ Q_LONG KExtendedSocket::writeBlock(const
       retval = KSocks::self()->write(sockfd, data, len);
       if (retval == -1)
 	setError(IO_WriteError, errno);
+      else
+	emit bytesWritten(retval);
     }
   else
     {
@@ -1680,6 +1701,19 @@ int KExtendedSocket::unreadBlock(const c
   return -1;
 }
 
+int KExtendedSocket::bytesAvailable() const
+{
+  if (d->status < connected || d->status >= closing || d->flags & passiveSocket)
+    return -2;
+
+  // as of now, we don't do any extra processing
+  // we only work in input-buffered sockets
+  if (d->flags * inputBufferedSocket)
+    return KBufferedIO::bytesAvailable();
+
+  return 0;			// TODO: FIONREAD ioctl
+}
+
 int KExtendedSocket::waitForMore(int msecs)
 {
   cleanError();
@@ -1709,9 +1743,9 @@ int KExtendedSocket::waitForMore(int mse
 
 int KExtendedSocket::getch()
 {
-  char c;
+  unsigned char c;
   int retval;
-  retval = readBlock(&c, sizeof(c));
+  retval = readBlock((char*)&c, sizeof(c));
 
   if (retval < 0)
     return retval;
@@ -1720,8 +1754,8 @@ int KExtendedSocket::getch()
 
 int KExtendedSocket::putch(int ch)
 {
-  char c = (char)ch;
-  return writeBlock(&c, sizeof(ch));
+  unsigned char c = (char)ch;
+  return writeBlock((char*)&c, sizeof(c));
 }
 
 int KExtendedSocket::doLookup(const QString &host, const QString &serv, addrinfo &hint,
@@ -1789,7 +1823,10 @@ void KExtendedSocket::enableWrite(bool e
 void KExtendedSocket::socketActivityRead()
 {
   if (d->flags & passiveSocket)
-    return;
+    {
+      emit readyAccept();
+      return;
+    }
   if (d->status == connecting)
     {
       connectionEvent();

["audiocd.patch" (text/x-diff)]

Index: kioslave/audiocd//cddb.cpp
===================================================================
RCS file: /home/kde/kdebase/kioslave/audiocd/cddb.cpp,v
retrieving revision 1.9
diff -u -3 -p -r1.9 cddb.cpp
--- kioslave/audiocd//cddb.cpp	2001/11/10 12:56:09	1.9
+++ kioslave/audiocd//cddb.cpp	2002/02/10 19:35:28
@@ -39,7 +39,7 @@
 #include "cddb.h"
 
 CDDB::CDDB()
-  : fd(0), port(0), remote(false), save_local(false)
+  : ks(0), port(0), remote(false), save_local(false)
 {
   cddb_dirs += ".cddb";
 }
@@ -52,7 +52,7 @@ CDDB::~CDDB()
 bool
 CDDB::set_server(const char *hostname, unsigned short int _port)
 {
-  if (fd)
+  if (ks)
     {
       if (h_name == hostname && port == _port)
         return true;
@@ -62,20 +62,12 @@ CDDB::set_server(const char *hostname, u
   kdDebug(7101) << "CDDB: set_server, host=" << hostname << "port=" << _port << endl;
   if (remote)
     {
-      ksockaddr_in addr;
-      memset(&addr, 0, sizeof(addr));
-      if (!KSocket::initSockaddr(&addr, hostname, _port))
-        return false;
-      if ((fd = ::socket(PF_INET, SOCK_STREAM, 0)) < 0)
-        {
-	  fd = 0;
-          return false;
-	}
-      if (::connect(fd, (struct sockaddr*)&addr, sizeof(addr)))
+      ks = new KExtendedSocket(hostname, _port);
+      if (ks->connect() < 0)
 	{
 	  kdDebug(7101) << "CDDB: Can't connect!" << endl;
-	  ::close(fd);
-	  fd = 0;
+	  delete ks;
+	  ks = 0;
           return false;
 	}
       
@@ -92,17 +84,17 @@ CDDB::set_server(const char *hostname, u
 bool
 CDDB::deinit()
 {
-  if (fd)
+  if (ks)
     {
       writeLine("quit");
       QCString r;
       readLine(r);
-      close (fd);
+      ks->close();
     }
   h_name.resize(0);
   port = 0;
   remote = false;
-  fd = 0;
+  ks = 0;
   return true;
 }
 
@@ -111,7 +103,7 @@ CDDB::readLine(QCString& ret)
 {
   int read_length = 0;
   char small_b[128];
-  fd_set set;
+  //fd_set set;
 
   ret.resize(0);
   while (read_length < 40000)
@@ -129,6 +121,8 @@ CDDB::readLine(QCString& ret)
 	  kdDebug(7101) << "CDDB: got  `" << ret << "'" << endl;
 	  return true;
 	}
+
+#if 0
       // Try to refill the buffer
       FD_ZERO(&set);
       FD_SET(fd, &set);
@@ -137,7 +131,11 @@ CDDB::readLine(QCString& ret)
       tv.tv_usec = 0;
       if (::select(fd+1, &set, 0, 0, &tv) < 0)
         return false;
-      ssize_t l = ::read(fd, &small_b, sizeof(small_b)-1);
+#endif
+
+      ks->waitForMore(60 * 1000);
+      ssize_t l = ks->readBlock(small_b, sizeof(small_b)-1);
+      //ssize_t l = ::read(fd, &small_b, sizeof(small_b)-1);
       if (l <= 0)
         {
 	  // l==0 normally means fd got closed, but we really need a lineend
@@ -158,7 +156,8 @@ CDDB::writeLine(const QCString& line)
   kdDebug(7101) << "CDDB: send `" << line << "'" << endl;
   while (l)
     {
-      ssize_t wl = ::write(fd, b, l);
+      //ssize_t wl = ::write(fd, b, l);
+      ssize_t wl = ks->writeBlock(b, l);
       if (wl < 0 && errno != EINTR)
         return false;
       if (wl < 0)
@@ -172,7 +171,8 @@ CDDB::writeLine(const QCString& line)
       char c = '\n';
       ssize_t wl;
       do {
-        wl = ::write(fd, &c, 1);
+        //wl = ::write(fd, &c, 1);
+	wl = ks->writeBlock(&c, 1);
       } while (wl <= 0 && errno == EINTR);
       if (wl<=0 && errno != EINTR)
         return false;
@@ -402,7 +402,7 @@ CDDB::queryCD(QValueList<int>& track_ofs
   /* First look for a local file.  */
   local = searchLocal (id, &file);
   /* If we have no local file, and no remote connection, barf.  */
-  if (!local && (!remote || fd == 0))
+  if (!local && (!remote || ks == 0))
     return false;
 
   m_tracks = num_tracks;
Index: kioslave/audiocd//cddb.h
===================================================================
RCS file: /home/kde/kdebase/kioslave/audiocd/cddb.h,v
retrieving revision 1.3
diff -u -3 -p -r1.3 cddb.h
--- kioslave/audiocd//cddb.h	2001/11/10 12:56:09	1.3
+++ kioslave/audiocd//cddb.h	2002/02/10 19:35:28
@@ -21,6 +21,7 @@
 #include <qcstring.h>
 #include <qvaluelist.h>
 #include <qstringlist.h>
+#include <kextsock.h>
 
 class QFile;
 class QTextStream;
@@ -43,7 +44,7 @@ private:
   bool deinit();
   bool parse_read_resp(QTextStream*, QTextStream*);
   bool searchLocal(unsigned int id, QFile *ret_file);
-  int fd;
+  KExtendedSocket *ks;
   QCString h_name;
   unsigned short int port;
   bool remote;


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic