[ 
https://issues.apache.org/jira/browse/THRIFT-3560?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16428281#comment-16428281
 ] 

ASF GitHub Bot commented on THRIFT-3560:
----------------------------------------

jeking3 closed pull request #798: THRIFT-3560: C++: declared 
TTransport::isOpen() and TTransport::getOrigin() as const member functions
URL: https://github.com/apache/thrift/pull/798
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/lib/cpp/src/thrift/qt/TQIODeviceTransport.cpp 
b/lib/cpp/src/thrift/qt/TQIODeviceTransport.cpp
index 686f242971..483b38a8c9 100644
--- a/lib/cpp/src/thrift/qt/TQIODeviceTransport.cpp
+++ b/lib/cpp/src/thrift/qt/TQIODeviceTransport.cpp
@@ -44,7 +44,7 @@ void TQIODeviceTransport::open() {
   }
 }
 
-bool TQIODeviceTransport::isOpen() {
+bool TQIODeviceTransport::isOpen() const {
   return dev_->isOpen();
 }
 
diff --git a/lib/cpp/src/thrift/qt/TQIODeviceTransport.h 
b/lib/cpp/src/thrift/qt/TQIODeviceTransport.h
index 8091d3287f..b80814b2db 100644
--- a/lib/cpp/src/thrift/qt/TQIODeviceTransport.h
+++ b/lib/cpp/src/thrift/qt/TQIODeviceTransport.h
@@ -40,7 +40,7 @@ class TQIODeviceTransport
   virtual ~TQIODeviceTransport();
 
   void open();
-  bool isOpen();
+  bool isOpen() const;
   bool peek();
   void close();
 
diff --git a/lib/cpp/src/thrift/transport/TBufferTransports.h 
b/lib/cpp/src/thrift/transport/TBufferTransports.h
index 013c6e0068..05a9442e1c 100644
--- a/lib/cpp/src/thrift/transport/TBufferTransports.h
+++ b/lib/cpp/src/thrift/transport/TBufferTransports.h
@@ -217,7 +217,7 @@ class TBufferedTransport : public 
TVirtualTransport<TBufferedTransport, TBufferB
 
   void open() { transport_->open(); }
 
-  bool isOpen() { return transport_->isOpen(); }
+  bool isOpen() const { return transport_->isOpen(); }
 
   bool peek() {
     if (rBase_ == rBound_) {
@@ -240,7 +240,7 @@ class TBufferedTransport : public 
TVirtualTransport<TBufferedTransport, TBufferB
   /**
    * Returns the origin of the underlying transport
    */
-  virtual const std::string getOrigin() { return transport_->getOrigin(); }
+  virtual const std::string getOrigin() const { return 
transport_->getOrigin(); }
 
   /**
    * The following behavior is currently implemented by TBufferedTransport,
@@ -345,7 +345,7 @@ class TFramedTransport : public 
TVirtualTransport<TFramedTransport, TBufferBase>
 
   void open() { transport_->open(); }
 
-  bool isOpen() { return transport_->isOpen(); }
+  bool isOpen() const { return transport_->isOpen(); }
 
   bool peek() { return (rBase_ < rBound_) || transport_->peek(); }
 
@@ -377,7 +377,7 @@ class TFramedTransport : public 
TVirtualTransport<TFramedTransport, TBufferBase>
   /**
    * Returns the origin of the underlying transport
    */
-  virtual const std::string getOrigin() { return transport_->getOrigin(); }
+  virtual const std::string getOrigin() const { return 
transport_->getOrigin(); }
 
   /**
    * Set the maximum size of the frame at read
@@ -547,7 +547,7 @@ class TMemoryBuffer : public 
TVirtualTransport<TMemoryBuffer, TBufferBase> {
     }
   }
 
-  bool isOpen() { return true; }
+  bool isOpen() const { return true; }
 
   bool peek() { return (rBase_ < wBase_); }
 
diff --git a/lib/cpp/src/thrift/transport/TFDTransport.h 
b/lib/cpp/src/thrift/transport/TFDTransport.h
index 5593d43dff..404927b897 100644
--- a/lib/cpp/src/thrift/transport/TFDTransport.h
+++ b/lib/cpp/src/thrift/transport/TFDTransport.h
@@ -53,7 +53,7 @@ class TFDTransport : public TVirtualTransport<TFDTransport> {
     }
   }
 
-  bool isOpen() { return fd_ >= 0; }
+  bool isOpen() const { return fd_ >= 0; }
 
   void open() {}
 
diff --git a/lib/cpp/src/thrift/transport/TFileTransport.h 
b/lib/cpp/src/thrift/transport/TFileTransport.h
index acd7bf9966..7a8d4219a9 100644
--- a/lib/cpp/src/thrift/transport/TFileTransport.h
+++ b/lib/cpp/src/thrift/transport/TFileTransport.h
@@ -180,7 +180,7 @@ class TFileTransport : public TFileReaderTransport, public 
TFileWriterTransport
 
   // TODO: what is the correct behaviour for this?
   // the log file is generally always open
-  bool isOpen() { return true; }
+  bool isOpen() const { return true; }
 
   void write(const uint8_t* buf, uint32_t len);
   void flush();
diff --git a/lib/cpp/src/thrift/transport/THeaderTransport.h 
b/lib/cpp/src/thrift/transport/THeaderTransport.h
index 94135ea5cd..e2809b1856 100644
--- a/lib/cpp/src/thrift/transport/THeaderTransport.h
+++ b/lib/cpp/src/thrift/transport/THeaderTransport.h
@@ -95,7 +95,7 @@ class THeaderTransport : public 
TVirtualTransport<THeaderTransport, TFramedTrans
 
   void open() { transport_->open(); }
 
-  bool isOpen() { return transport_->isOpen(); }
+  bool isOpen() const { return transport_->isOpen(); }
 
   bool peek() { return (this->rBase_ < this->rBound_) || transport_->peek(); }
 
diff --git a/lib/cpp/src/thrift/transport/THttpTransport.cpp 
b/lib/cpp/src/thrift/transport/THttpTransport.cpp
index a466ff62a1..0be82e7ef7 100644
--- a/lib/cpp/src/thrift/transport/THttpTransport.cpp
+++ b/lib/cpp/src/thrift/transport/THttpTransport.cpp
@@ -254,7 +254,7 @@ void THttpTransport::write(const uint8_t* buf, uint32_t 
len) {
   writeBuffer_.write(buf, len);
 }
 
-const std::string THttpTransport::getOrigin() {
+const std::string THttpTransport::getOrigin() const {
   std::ostringstream oss;
   if (!origin_.empty()) {
     oss << origin_ << ", ";
diff --git a/lib/cpp/src/thrift/transport/THttpTransport.h 
b/lib/cpp/src/thrift/transport/THttpTransport.h
index a9f564c481..cc16a7614a 100644
--- a/lib/cpp/src/thrift/transport/THttpTransport.h
+++ b/lib/cpp/src/thrift/transport/THttpTransport.h
@@ -42,7 +42,7 @@ class THttpTransport : public 
TVirtualTransport<THttpTransport> {
 
   void open() { transport_->open(); }
 
-  bool isOpen() { return transport_->isOpen(); }
+  bool isOpen() const { return transport_->isOpen(); }
 
   bool peek() { return transport_->peek(); }
 
@@ -56,7 +56,7 @@ class THttpTransport : public 
TVirtualTransport<THttpTransport> {
 
   virtual void flush() = 0;
 
-  virtual const std::string getOrigin();
+  virtual const std::string getOrigin() const;
 
 protected:
   boost::shared_ptr<TTransport> transport_;
diff --git a/lib/cpp/src/thrift/transport/TPipe.cpp 
b/lib/cpp/src/thrift/transport/TPipe.cpp
index 75ce5d2a42..3843f254b6 100644
--- a/lib/cpp/src/thrift/transport/TPipe.cpp
+++ b/lib/cpp/src/thrift/transport/TPipe.cpp
@@ -255,7 +255,7 @@ TPipe::~TPipe() {
 //---------------------------------------------------------
 // Transport callbacks
 //---------------------------------------------------------
-bool TPipe::isOpen() {
+bool TPipe::isOpen() const {
   return impl_.get() != NULL;
 }
 
diff --git a/lib/cpp/src/thrift/transport/TPipe.h 
b/lib/cpp/src/thrift/transport/TPipe.h
index 5dd8f9a241..f87cd050fe 100644
--- a/lib/cpp/src/thrift/transport/TPipe.h
+++ b/lib/cpp/src/thrift/transport/TPipe.h
@@ -63,7 +63,7 @@ class TPipe : public TVirtualTransport<TPipe> {
   virtual ~TPipe();
 
   // Returns whether the pipe is open & valid.
-  virtual bool isOpen();
+  virtual bool isOpen() const;
 
   // Checks whether more data is available in the pipe.
   virtual bool peek();
diff --git a/lib/cpp/src/thrift/transport/TSSLSocket.cpp 
b/lib/cpp/src/thrift/transport/TSSLSocket.cpp
index 8e56426784..dcde49b774 100644
--- a/lib/cpp/src/thrift/transport/TSSLSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TSSLSocket.cpp
@@ -227,7 +227,7 @@ TSSLSocket::~TSSLSocket() {
   close();
 }
 
-bool TSSLSocket::isOpen() {
+bool TSSLSocket::isOpen() const {
   if (ssl_ == NULL || !TSocket::isOpen()) {
     return false;
   }
diff --git a/lib/cpp/src/thrift/transport/TSSLSocket.h 
b/lib/cpp/src/thrift/transport/TSSLSocket.h
index ba8abf418b..413114c840 100644
--- a/lib/cpp/src/thrift/transport/TSSLSocket.h
+++ b/lib/cpp/src/thrift/transport/TSSLSocket.h
@@ -72,7 +72,7 @@ class TSSLSocket : public TSocket {
   /**
    * TTransport interface.
    */
-  bool isOpen();
+  bool isOpen() const;
   bool peek();
   void open();
   void close();
diff --git a/lib/cpp/src/thrift/transport/TShortReadTransport.h 
b/lib/cpp/src/thrift/transport/TShortReadTransport.h
index f2ecae132d..86a5e2e983 100644
--- a/lib/cpp/src/thrift/transport/TShortReadTransport.h
+++ b/lib/cpp/src/thrift/transport/TShortReadTransport.h
@@ -41,7 +41,7 @@ class TShortReadTransport : public 
TVirtualTransport<TShortReadTransport> {
   TShortReadTransport(boost::shared_ptr<TTransport> transport, double 
full_prob)
     : transport_(transport), fullProb_(full_prob) {}
 
-  bool isOpen() { return transport_->isOpen(); }
+  bool isOpen() const { return transport_->isOpen(); }
 
   bool peek() { return transport_->peek(); }
 
diff --git a/lib/cpp/src/thrift/transport/TSocket.cpp 
b/lib/cpp/src/thrift/transport/TSocket.cpp
index 6de6d4ebb6..fee3bbcb4e 100644
--- a/lib/cpp/src/thrift/transport/TSocket.cpp
+++ b/lib/cpp/src/thrift/transport/TSocket.cpp
@@ -174,7 +174,7 @@ TSocket::~TSocket() {
   close();
 }
 
-bool TSocket::isOpen() {
+bool TSocket::isOpen() const {
   return (socket_ != THRIFT_INVALID_SOCKET);
 }
 
@@ -674,11 +674,11 @@ uint32_t TSocket::write_partial(const uint8_t* buf, 
uint32_t len) {
   return b;
 }
 
-std::string TSocket::getHost() {
+std::string TSocket::getHost() const {
   return host_;
 }
 
-int TSocket::getPort() {
+int TSocket::getPort() const {
   return port_;
 }
 
@@ -789,7 +789,7 @@ void TSocket::setMaxRecvRetries(int maxRecvRetries) {
   maxRecvRetries_ = maxRecvRetries;
 }
 
-string TSocket::getSocketInfo() {
+string TSocket::getSocketInfo() const {
   std::ostringstream oss;
   if (host_.empty() || port_ == 0) {
     oss << "<Host: " << getPeerAddress();
@@ -800,7 +800,7 @@ string TSocket::getSocketInfo() {
   return oss.str();
 }
 
-std::string TSocket::getPeerHost() {
+std::string TSocket::getPeerHost() const {
   if (peerHost_.empty() && path_.empty()) {
     struct sockaddr_storage addr;
     struct sockaddr* addrPtr;
@@ -819,7 +819,7 @@ std::string TSocket::getPeerHost() {
       }
       addrPtr = (sockaddr*)&addr;
 
-      setCachedAddress(addrPtr, addrLen);
+      const_cast<TSocket*>(this)->setCachedAddress(addrPtr, addrLen);
     }
 
     char clienthost[NI_MAXHOST];
@@ -838,7 +838,7 @@ std::string TSocket::getPeerHost() {
   return peerHost_;
 }
 
-std::string TSocket::getPeerAddress() {
+std::string TSocket::getPeerAddress() const {
   if (peerAddress_.empty() && path_.empty()) {
     struct sockaddr_storage addr;
     struct sockaddr* addrPtr;
@@ -857,7 +857,7 @@ std::string TSocket::getPeerAddress() {
       }
       addrPtr = (sockaddr*)&addr;
 
-      setCachedAddress(addrPtr, addrLen);
+      const_cast<TSocket*>(this)->setCachedAddress(addrPtr, addrLen);
     }
 
     char clienthost[NI_MAXHOST];
@@ -877,7 +877,7 @@ std::string TSocket::getPeerAddress() {
   return peerAddress_;
 }
 
-int TSocket::getPeerPort() {
+int TSocket::getPeerPort() const {
   getPeerAddress();
   return peerPort_;
 }
@@ -927,7 +927,7 @@ bool TSocket::getUseLowMinRto() {
   return useLowMinRto_;
 }
 
-const std::string TSocket::getOrigin() {
+const std::string TSocket::getOrigin() const {
   std::ostringstream oss;
   oss << getPeerHost() << ":" << getPeerPort();
   return oss.str();
diff --git a/lib/cpp/src/thrift/transport/TSocket.h 
b/lib/cpp/src/thrift/transport/TSocket.h
index 9f0074d145..f9a62b12bb 100644
--- a/lib/cpp/src/thrift/transport/TSocket.h
+++ b/lib/cpp/src/thrift/transport/TSocket.h
@@ -81,7 +81,7 @@ class TSocket : public TVirtualTransport<TSocket> {
    *
    * @return Is the socket alive?
    */
-  virtual bool isOpen();
+  virtual bool isOpen() const;
 
   /**
    * Calls select on the socket to see if there is more data available.
@@ -127,14 +127,14 @@ class TSocket : public TVirtualTransport<TSocket> {
    *
    * @return string host identifier
    */
-  std::string getHost();
+  std::string getHost() const;
 
   /**
    * Get the port that the socket is connected to
    *
    * @return int port number
    */
-  int getPort();
+  int getPort() const;
 
   /**
    * Set the host that socket will connect to
@@ -195,27 +195,27 @@ class TSocket : public TVirtualTransport<TSocket> {
   /**
    * Get socket information formatted as a string <Host: x Port: x>
    */
-  std::string getSocketInfo();
+  std::string getSocketInfo() const;
 
   /**
    * Returns the DNS name of the host to which the socket is connected
    */
-  std::string getPeerHost();
+  std::string getPeerHost() const;
 
   /**
    * Returns the address of the host to which the socket is connected
    */
-  std::string getPeerAddress();
+  std::string getPeerAddress() const;
 
   /**
    * Returns the port of the host to which the socket is connected
    **/
-  int getPeerPort();
+  int getPeerPort() const;
 
   /**
    * Returns the underlying socket file descriptor.
    */
-  THRIFT_SOCKET getSocketFD() { return socket_; }
+  THRIFT_SOCKET getSocketFD() const { return socket_; }
 
   /**
    * (Re-)initialize a TSocket for the supplied descriptor.  This is only
@@ -226,7 +226,7 @@ class TSocket : public TVirtualTransport<TSocket> {
    */
   void setSocketFD(THRIFT_SOCKET fd);
 
-  /*
+  /**
    * Returns a cached copy of the peer address.
    */
   sockaddr* getCachedAddress(socklen_t* len) const;
@@ -246,7 +246,7 @@ class TSocket : public TVirtualTransport<TSocket> {
    *
    * @return string peer host identifier and port
    */
-  virtual const std::string getOrigin();
+  virtual const std::string getOrigin() const;
 
   /**
    * Constructor to create socket from file descriptor.
@@ -273,13 +273,13 @@ class TSocket : public TVirtualTransport<TSocket> {
   std::string host_;
 
   /** Peer hostname */
-  std::string peerHost_;
+  mutable std::string peerHost_;
 
   /** Peer address */
-  std::string peerAddress_;
+  mutable std::string peerAddress_;
 
   /** Peer port */
-  int peerPort_;
+  mutable int peerPort_;
 
   /** Port number to connect on */
   int port_;
diff --git a/lib/cpp/src/thrift/transport/TTransport.h 
b/lib/cpp/src/thrift/transport/TTransport.h
index d06b0f8829..1906f5d126 100644
--- a/lib/cpp/src/thrift/transport/TTransport.h
+++ b/lib/cpp/src/thrift/transport/TTransport.h
@@ -63,7 +63,7 @@ class TTransport {
   /**
    * Whether this transport is open.
    */
-  virtual bool isOpen() { return false; }
+  virtual bool isOpen() const { return false; }
 
   /**
    * Tests whether there is more data to read or if the remote side is
@@ -236,7 +236,7 @@ class TTransport {
    *
    * The returned value can be used in a log message for example
    */
-  virtual const std::string getOrigin() { return "Unknown"; }
+  virtual const std::string getOrigin() const { return "Unknown"; }
 
 protected:
   /**
diff --git a/lib/cpp/src/thrift/transport/TTransportUtils.cpp 
b/lib/cpp/src/thrift/transport/TTransportUtils.cpp
index 0f24c95be4..a8365356c2 100644
--- a/lib/cpp/src/thrift/transport/TTransportUtils.cpp
+++ b/lib/cpp/src/thrift/transport/TTransportUtils.cpp
@@ -102,7 +102,7 @@ TPipedFileReaderTransport::TPipedFileReaderTransport(
 TPipedFileReaderTransport::~TPipedFileReaderTransport() {
 }
 
-bool TPipedFileReaderTransport::isOpen() {
+bool TPipedFileReaderTransport::isOpen() const {
   return TPipedTransport::isOpen();
 }
 
diff --git a/lib/cpp/src/thrift/transport/TTransportUtils.h 
b/lib/cpp/src/thrift/transport/TTransportUtils.h
index c22183650a..c2aefeaf72 100644
--- a/lib/cpp/src/thrift/transport/TTransportUtils.h
+++ b/lib/cpp/src/thrift/transport/TTransportUtils.h
@@ -46,7 +46,7 @@ class TNullTransport : public 
TVirtualTransport<TNullTransport> {
 
   ~TNullTransport() {}
 
-  bool isOpen() { return true; }
+  bool isOpen() const { return true; }
 
   void open() {}
 
@@ -112,7 +112,7 @@ class TPipedTransport : virtual public TTransport {
     std::free(wBuf_);
   }
 
-  bool isOpen() { return srcTrans_->isOpen(); }
+  bool isOpen() const { return srcTrans_->isOpen(); }
 
   bool peek() {
     if (rPos_ >= rLen_) {
@@ -242,7 +242,7 @@ class TPipedFileReaderTransport : public TPipedTransport, 
public TFileReaderTran
   ~TPipedFileReaderTransport();
 
   // TTransport functions
-  bool isOpen();
+  bool isOpen() const;
   bool peek();
   void open();
   void close();
diff --git a/lib/cpp/src/thrift/transport/TZlibTransport.cpp 
b/lib/cpp/src/thrift/transport/TZlibTransport.cpp
index fb5cc5da16..106a657b97 100644
--- a/lib/cpp/src/thrift/transport/TZlibTransport.cpp
+++ b/lib/cpp/src/thrift/transport/TZlibTransport.cpp
@@ -110,7 +110,7 @@ TZlibTransport::~TZlibTransport() {
   delete wstream_;
 }
 
-bool TZlibTransport::isOpen() {
+bool TZlibTransport::isOpen() const {
   return (readAvail() > 0) || (rstream_->avail_in > 0) || transport_->isOpen();
 }
 
@@ -131,7 +131,7 @@ bool TZlibTransport::peek() {
 // In standalone objects, we set input_ended_ to true when inflate returns
 // Z_STREAM_END.  This allows to make sure that a checksum was verified.
 
-inline int TZlibTransport::readAvail() {
+inline int TZlibTransport::readAvail() const {
   return urbuf_size_ - rstream_->avail_out - urpos_;
 }
 
diff --git a/lib/cpp/src/thrift/transport/TZlibTransport.h 
b/lib/cpp/src/thrift/transport/TZlibTransport.h
index 1e7b5ec5e6..2120e81576 100644
--- a/lib/cpp/src/thrift/transport/TZlibTransport.h
+++ b/lib/cpp/src/thrift/transport/TZlibTransport.h
@@ -138,7 +138,7 @@ class TZlibTransport : public 
TVirtualTransport<TZlibTransport> {
    */
   ~TZlibTransport();
 
-  bool isOpen();
+  bool isOpen() const;
   bool peek();
 
   void open() { transport_->open(); }
@@ -183,7 +183,7 @@ class TZlibTransport : public 
TVirtualTransport<TZlibTransport> {
 protected:
   inline void checkZlibRv(int status, const char* msg);
   inline void checkZlibRvNothrow(int status, const char* msg);
-  inline int readAvail();
+  inline int readAvail() const;
   void flushToTransport(int flush);
   void flushToZlib(const uint8_t* buf, int len, int flush);
   bool readFromZlib();


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> C++ TTransport::isOpen() and TTransport::getOrigin() should be const member 
> functions
> -------------------------------------------------------------------------------------
>
>                 Key: THRIFT-3560
>                 URL: https://issues.apache.org/jira/browse/THRIFT-3560
>             Project: Thrift
>          Issue Type: Improvement
>          Components: C++ - Library
>    Affects Versions: 0.9.3, 0.10.0, 1.0
>            Reporter: Sebastian Zenker
>            Priority: Minor
>              Labels: easyfix
>
> ... as they should not alter the state of TTransport. Also peek() should be 
> const in my opinion, but this requires some more refactoring.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to