From 6bfdc232d146ed7bf2eb075984dc59b5e51dc39f Mon Sep 17 00:00:00 2001 From: dockes Date: Fri, 9 Oct 2009 13:34:18 +0000 Subject: [PATCH] Execmd: added count parameter to receive(), and new getline() function Netcon: fix receive() to properly handle the case where there is initially data in the line buffer --- src/utils/execmd.cpp | 47 ++++++++++++++++++++++++++++++++++++++------ src/utils/execmd.h | 5 ++++- src/utils/netcon.cpp | 43 +++++++++++++++++++++------------------- src/utils/netcon.h | 45 +++++++++++++++++++++++++++--------------- 4 files changed, 97 insertions(+), 43 deletions(-) diff --git a/src/utils/execmd.cpp b/src/utils/execmd.cpp index 3424c0e0..bc5bc1b5 100644 --- a/src/utils/execmd.cpp +++ b/src/utils/execmd.cpp @@ -51,7 +51,12 @@ static char rcsid[] = "@(#$Id: execmd.cpp,v 1.27 2008-10-06 06:22:47 dockes Exp using namespace std; #endif /* NO_NAMESPACES */ +#ifndef MAX #define MAX(A,B) ((A) > (B) ? (A) : (B)) +#endif +#ifndef MIN +#define MIN(A,B) ((A) < (B) ? (A) : (B)) +#endif /* From FreeBSD's which command */ static bool @@ -373,19 +378,49 @@ int ExecCmd::send(const string& data) return nwritten; } -int ExecCmd::receive(string& data) +int ExecCmd::receive(string& data, int cnt) { NetconCli *con = dynamic_cast(m_fromcmd.getptr()); if (con == 0) { - LOGERR(("ExecCmd::receive: outpipe is closed\n")); + LOGERR(("ExecCmd::receive: inpipe is closed\n")); return -1; } - char buf[8192]; - int n = con->receive(buf, 8192); + const int BS = 4096; + char buf[BS]; + int ntot = 0; + do { + int toread = cnt > 0 ? MIN(cnt - ntot, BS) : BS; + int n = con->receive(buf, toread); + if (n < 0) { + LOGERR(("ExecCmd::receive: error\n")); + return -1; + } else if (n > 0) { + ntot += n; + data.append(buf, n); + } else { + LOGDEB(("ExecCmd::receive: got 0\n")); + break; + } + } while (cnt > 0 && ntot < cnt); + return ntot; +} + +int ExecCmd::getline(string& data) +{ + NetconCli *con = dynamic_cast(m_fromcmd.getptr()); + if (con == 0) { + LOGERR(("ExecCmd::receive: inpipe is closed\n")); + return -1; + } + const int BS = 1024; + char buf[BS]; + int n = con->getline(buf, BS); if (n < 0) { - LOGERR(("ExecCmd::receive: error\n")); + LOGERR(("ExecCmd::getline: error\n")); } else if (n > 0) { data.append(buf, n); + } else { + LOGDEB(("ExecCmd::getline: got 0\n")); } return n; } @@ -395,7 +430,7 @@ int ExecCmd::wait(bool haderror) { ExecCmdRsrc e(this); int status = -1; - if (!m_cancelRequest) { + if (!m_cancelRequest && m_pid > 0) { if (waitpid(m_pid, &status, 0) < 0) status = -1; m_pid = -1; diff --git a/src/utils/execmd.h b/src/utils/execmd.h index 4a1b7fd2..a5a38379 100644 --- a/src/utils/execmd.h +++ b/src/utils/execmd.h @@ -134,9 +134,12 @@ class ExecCmd { int startExec(const string &cmd, const list& args, bool has_input, bool has_output); int send(const string& data); - int receive(string& data); + int receive(string& data, int cnt = -1); + int getline(string& data); int wait(bool haderror = false); + pid_t getChildPid() {return m_pid;} + /** * Cancel/kill command. This can be called from another thread or * from the advise callback, which could also raise an exception to diff --git a/src/utils/netcon.cpp b/src/utils/netcon.cpp index ecdd1fd8..757ae3f6 100644 --- a/src/utils/netcon.cpp +++ b/src/utils/netcon.cpp @@ -59,6 +59,7 @@ static const int zero = 0; #define MILLIS(OLD, NEW) ( (long)(((NEW).tv_sec - (OLD).tv_sec) * 1000 + \ ((NEW).tv_usec - (OLD).tv_usec) / 1000)) +// Static method // Simplified interface to 'select()'. Only use one fd, for either // reading or writing. This is only used when not using the // selectloop() style of network i/o. @@ -116,9 +117,9 @@ void Netcon::setperiodichandler(int (*handler)(void *), void *p, int ms) gettimeofday(&lasthdlcall, 0); } -// set the appropriate timeout so that the select call returns in time -// to call the periodic routine. -void periodictimeout(struct timeval *tv) +// Compute the appropriate timeout so that the select call returns in +// time to call the periodic routine. +static void periodictimeout(struct timeval *tv) { // If periodic not set, the select call times out and we loop // after a very long time (we'd need to pass NULL to select for an @@ -143,7 +144,7 @@ void periodictimeout(struct timeval *tv) // Check if it's time to call the handler. selectloop will return to // caller if it or we return 0 -int maybecallperiodic() +static int maybecallperiodic() { if (periodicmillis <= 0) return 1; @@ -310,10 +311,11 @@ Netcon::~Netcon() { void Netcon::closeconn() { - if (m_fd >= 0) { + if (m_ownfd && m_fd >= 0) { close(m_fd); - m_fd = -1; } + m_fd = -1; + m_ownfd = true; } char *Netcon::sterror() @@ -418,22 +420,24 @@ int NetconData::writeready() // Receive at most cnt bytes (maybe less) int NetconData::receive(char *buf, int cnt, int timeo) { - LOGDEB2(("NetconData::receive: cnt %d timeo %d m_buf 0x%x m_bufbytes %d\n", + LOGDEB2(("NetconData::receive: cnt %d timeo %d m_buf 0x%x m_bufbytes %d\n", cnt, timeo, m_buf, m_bufbytes)); if (m_fd < 0) { LOGERR(("NetconData::receive: connection not opened\n")); return -1; } + int fromibuf = 0; // Get whatever might have been left in the buffer by a previous // getline, except if we're called to fill the buffer of course if (m_buf && m_bufbytes > 0 && (buf < m_buf || buf > m_buf + m_bufsize)) { - int frombuf = MIN(m_bufbytes, cnt); - memcpy(buf, m_bufbase, frombuf); - m_bufbytes -= frombuf; - m_bufbase += frombuf; - cnt -= frombuf; + fromibuf = MIN(m_bufbytes, cnt); + memcpy(buf, m_bufbase, fromibuf); + m_bufbytes -= fromibuf; + m_bufbase += fromibuf; + cnt -= fromibuf; + LOGDEB2(("NetconData::receive: transferred %d from mbuf\n", fromibuf)); if (cnt <= 0) - return frombuf; + return fromibuf; } if (timeo > 0) { int ret = select1(m_fd, timeo); @@ -448,14 +452,13 @@ int NetconData::receive(char *buf, int cnt, int timeo) } } m_didtimo = 0; - int flags = 0; - if ((cnt = read(m_fd, buf, cnt)) < 0) { + if ((cnt = read(m_fd, buf + fromibuf, cnt)) < 0) { char fdcbuf[10];sprintf(fdcbuf, "%d", m_fd); LOGSYSERR("NetconData::receive", "read", fdcbuf); return -1; } LOGDEB2(("NetconData::receive: normal return, cnt %d\n", cnt)); - return cnt; + return fromibuf + cnt; } // Receive exactly cnt bytes (except for timeout) @@ -510,9 +513,9 @@ int NetconData::getline(char *buf, int cnt, int timeo) LOGDEB2(("Before loop, bufbytes %d, maxtransf %d, nn: %d\n", m_bufbytes, maxtransf, nn)); for (nn = maxtransf; nn > 0;) { - // This is not pretty but we want nn to be decremented for each - // byte copied (even newline), and not become -1 if we go to the end - // Better ways welcome! + // This is not pretty but we want nn to be decremented for + // each byte copied (even newline), and not become -1 if + // we go to the end. Better ways welcome! nn--; if ((*cp++ = *m_bufbase++) == '\n') break; @@ -657,10 +660,10 @@ int NetconCli::openconn(const char *host, char *serv, int timeo) int NetconCli::setconn(int fd) { LOGDEB2(("Netconcli::setconn: fd %d\n", fd)); - closeconn(); m_fd = fd; + m_ownfd = false; setpeer(""); return 0; diff --git a/src/utils/netcon.h b/src/utils/netcon.h index 6d5402c6..47aa0381 100644 --- a/src/utils/netcon.h +++ b/src/utils/netcon.h @@ -6,12 +6,15 @@ /// A set of classes to manage client-server communication over a /// connection-oriented network, or a pipe. /// -/// Currently only uses TCP. The classes include client-side and -/// server-side (accepting) endpoints, and server-side code to handle -/// a set of client connections in parallel. +/// The listening/connection-accepting code currently only uses +/// TCP. The classes include client-side and server-side (accepting) +/// endpoints. Netcon also has server-side static code to handle a set +/// of client connections in parallel. This should be moved to a +/// friend class. /// /// The client data transfer class can also be used for -/// timeout-protected/asynchronous io using a given fd. +/// timeout-protected/asynchronous io using a given fd (ie a pipe +/// descriptor) /// Base class for all network endpoints: class Netcon; @@ -21,7 +24,7 @@ class Netcon { public: enum Event {NETCONPOLL_READ = 0x1, NETCONPOLL_WRITE=0x2}; Netcon() - : m_peer(0), m_fd(-1), m_didtimo(0), m_wantedEvents(0) + : m_peer(0), m_fd(-1), m_ownfd(true), m_didtimo(0), m_wantedEvents(0) {} virtual ~Netcon(); /// Remember whom we're talking to. We let external code do this because @@ -65,7 +68,7 @@ public: /// All the interface is static (independant of any given object). /// Loop waiting for events on the connections and call the - /// cando() method on the object where something happens (this will in + /// cando() method on the object when something happens (this will in /// turn typically call the app callback set on the netcon). Possibly /// call the periodic handler (if set) at regular intervals. /// @return -1 for error. 0 if no descriptors left for i/o. 1 for periodic @@ -89,19 +92,20 @@ public: /// @param handler the function to be called. /// - if it is 0, selectloop() will return after ms mS (and can be called /// again - /// - if it is not 0, it will be called at ms mS intervals. If its return - /// value is <= 0, selectloop will return. + /// - if it is not 0, it will be called at ms mS intervals. If its return + /// value is <= 0, selectloop will return. /// @param clp client data to be passed to handler at every call. - /// @param ms milliseconds interval between handler calls or before return. - /// set ms to 0 for no periodic handler + /// @param ms milliseconds interval between handler calls or + /// before return. Set to 0 for no periodic handler. static void setperiodichandler(int (*handler)(void *), void *clp, int ms); protected: static bool o_selectloopDoReturn; static int o_selectloopReturnValue; char *m_peer; // Name of the connected host - int m_fd; - int m_didtimo; + int m_fd; + bool m_ownfd; + int m_didtimo; // Used when part of the selectloop map. short m_wantedEvents; // Method called by the selectloop when something can be done with a netcon @@ -125,7 +129,7 @@ public: virtual int data(NetconData *con, Netcon::Event reason) = 0; }; -/// Base class for connections that actually transfer data. +/// Base class for connections that actually transfer data. T class NetconData : public Netcon { public: NetconData() : m_buf(0), m_bufbase(0), m_bufbytes(0), m_bufsize(0) @@ -142,7 +146,8 @@ public: /// Read from the connection /// @param buf the data buffer - /// @param cnt the number of bytes we should try to read + /// @param cnt the number of bytes we should try to read (but we return + /// as soon as we get data) /// @param timeo maximum number of seconds we should be waiting for data. /// @return the count of bytes actually read. 0 for timeout (call /// didtimo() to discriminate from EOF). -1 if an error occurred. @@ -172,16 +177,24 @@ private: class NetconCli : public NetconData { public: NetconCli(int silent = 0) {m_silentconnectfailure = silent;} + /// Open connection to specified host and named service. int openconn(const char *host, char *serv, int timeo = -1); + /// Open connection to specified host and numeric port. port is in /// HOST byte order int openconn(const char *host, unsigned int port, int timeo = -1); - /// Reuse existing fd. We DONT take ownership of the fd, and do no closin' - /// EVEN on an explicit closeconn() (use getfd(), close, setconn(-1)). + + /// Reuse existing fd. + /// We DONT take ownership of the fd, and do no closin' EVEN on an + /// explicit closeconn() or setconn() (use getfd(), close, + /// setconn(-1) if you need to really close the fd and have no + /// other copy). int setconn(int fd); + /// Do not log message if openconn() fails. void setSilentFail(int onoff) {m_silentconnectfailure = onoff;} + private: int m_silentconnectfailure; // No logging of connection failures if set };