Try to complete all TODO's #2

tkhurana96 · 2017-02-17T16:35:59Z

No description provided.

agauniyal · 2017-02-17T17:48:13Z

include/socket_family.hpp

 namespace method {

-	inline std::string getErrorMsg()
+	inline std::string getErrorMsg(int errorNumber)


const int errorNumber

agauniyal · 2017-02-17T17:52:36Z

src/socket.cpp


-	if (res == -1) {
-		throw std::runtime_error(net::method::getErrorMsg());
+	if (res == -1 && errno != EINPROGRESS ) {


take out errno and use that for checks, send that into net::method::getErrorMsg(errno) too.

Also, needs #include <errno.h> for errno - http://man7.org/linux/man-pages/man3/errno.3.html

agauniyal · 2017-02-17T17:52:58Z

src/socket.cpp

 	auto client = ::accept(sockfd, (sockaddr *) peerAddr.get(), &peerAddrSize);

-	if (client == -1) {
+	if (client == -1 && errno != EAGAIN && errno != EWOULDBLOCK) {


same here, take out errno into another var.

agauniyal · 2017-02-17T17:53:14Z

src/socket.cpp


-	if (client == -1) {
+	if (client == -1 && errno != EAGAIN && errno != EWOULDBLOCK) {
 		// TODO: Check for client == EAGAIN or EWOULDBLOCK and


delete these comments.

…Blocking in read and recv

agauniyal · 2017-02-19T06:39:25Z

src/socket.cpp

+	const auto currentErrNo = errno;
+	errno                   = 0;
+	if (client == -1 && currentErrNo != EAGAIN && currentErrNo != EWOULDBLOCK) {
+		throw std::runtime_error(net::method::getErrorMsg(currentErrNo));


No way of finding out if non-blocking error.

agauniyal · 2017-02-19T06:40:27Z

src/socket.cpp

+	const auto currentErrNo = errno;
+	errno                   = 0;
+	if (written == -1 && currentErrNo != EAGAIN && currentErrNo != EWOULDBLOCK) {
+		throw std::runtime_error(net::method::getErrorMsg(currentErrNo));


No way of finding out if non-blocking error.

agauniyal · 2017-02-19T06:41:20Z

src/socket.cpp

+	const auto currentErrNo = errno;
+	errno                   = 0;
+	if (sent == -1 && currentErrNo != EAGAIN && currentErrNo != EWOULDBLOCK) {
+		throw std::runtime_error(net::method::getErrorMsg(currentErrNo));


No way of finding out if non-blocking error.

agauniyal · 2017-02-19T06:43:15Z

src/socket.cpp

+	const auto currentErrNo = errno;
+	errno                   = 0;
+	if (sent == -1 && currentErrNo != EAGAIN && currentErrNo != EWOULDBLOCK) {
+		throw std::runtime_error(net::method::getErrorMsg(currentErrNo));


No way of finding out if non-blocking error.

agauniyal · 2017-02-19T06:44:07Z

src/socket.cpp



-std::string Socket::read(const int _bufSize) const
+std::string Socket::read(const int _bufSize, bool *_isSpecialErrorInNonBlocking) const


parameter name quite big... _ErrorNB seems fine.

agauniyal · 2017-02-19T06:44:55Z

src/socket.cpp

-		throw std::runtime_error(net::method::getErrorMsg());
+		if ( !_isSpecialErrorInNonBlocking || (_isSpecialErrorInNonBlocking && currentErrNo != EAGAIN && currentErrNo != EWOULDBLOCK))	// blocking & non blocking failure case
+			throw std::runtime_error(net::method::getErrorMsg(currentErrNo));
+		// else if ( _isSpecialErrorInNonBlocking && currentErrNo != EAGAIN && currentErrNo != EWOULDBLOCK) // non blocking failure case


remove comments, use proper braces { } for if else statements.

simplify logic, let compiler do the work.

Does not throws invalid_argument in case nullptr is sent/or last param is ignored and non-blocking error occurs.

agauniyal · 2017-02-19T06:47:06Z

src/socket.cpp

-		// TODO: Check for bytes == EAGAIN or EWOULDBLOCK and
-		// don't throw exception in that case.
-		throw std::runtime_error(net::method::getErrorMsg());
+		if ( !_isSpecialErrorInNonBlocking || (_isSpecialErrorInNonBlocking && currentErrNo != EAGAIN && currentErrNo != EWOULDBLOCK))	// blocking & non blocking failure case


explicitly check for != nullptr instead of executing bool operator. Makes intent clear.

agauniyal · 2017-02-19T06:49:33Z

src/socket.cpp

-		// TODO: Check for recvd == EAGAIN or EWOULDBLOCK and
-		// don't throw exception in that case.
-		throw std::runtime_error(net::method::getErrorMsg());
+		if ( !_isSpecialErrorInNonBlocking || (_isSpecialErrorInNonBlocking && currentErrNo != EAGAIN && currentErrNo != EWOULDBLOCK))


tkhurana96 added 2 commits February 17, 2017 21:53

Try to complete all TODO's

de7b462

Remove static_cast from errno comparisons

7a0ce37

tkhurana96 self-assigned this Feb 17, 2017

tkhurana96 requested a review from agauniyal February 17, 2017 16:35

agauniyal suggested changes Feb 17, 2017

View reviewed changes

tkhurana96 added 4 commits February 19, 2017 03:48

Add code for capturing current errno value in currentErrNo variable

fa46ced

Try to handle error in non-blocking sockets using bool *_isErrorInNon…

b754115

…Blocking in read and recv

Make getErrorMsg func arg const

89cfdc6

Rename isErrorInNonBlocking to isSpecialErrorInNonBlocking

df020a7

agauniyal suggested changes Feb 19, 2017

View reviewed changes

Refactor error handling, add initial support for AF_UNIX

8fc6948

agauniyal approved these changes Feb 19, 2017

View reviewed changes

agauniyal merged commit f3d9d33 into feature/socket Feb 19, 2017

agauniyal deleted the errorHandling branch February 19, 2017 09:03

agauniyal added the enhancement label Feb 23, 2017

agauniyal added this to the 0.1 milestone Feb 23, 2017



		std::string Socket::read(const int _bufSize) const
		std::string Socket::read(const int _bufSize, bool *_isSpecialErrorInNonBlocking) const

Try to complete all TODO's #2

Try to complete all TODO's #2

Uh oh!

Conversation

tkhurana96 commented Feb 17, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants