Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unit tests and bug fixes for XmlRpcSocket #1218

Merged
merged 4 commits into from
Nov 7, 2017
Merged

Unit tests and bug fixes for XmlRpcSocket #1218

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b2e3ab2
Unit tests and bug fixes for XmlRpcSocket
trainman419 Nov 3, 2017
8e24ee4
Add LGPL copyright block and add myself to authors.
trainman419 Nov 3, 2017
4ed28f7
Wrap features that obviously won't work on Windows
trainman419 Nov 7, 2017
d9320f2
Indent
trainman419 Nov 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions utilities/xmlrpcpp/include/xmlrpcpp/XmlRpcSocket.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ namespace XmlRpc {
static bool nbRead(int socket, std::string& s, bool *eof);

//! Write text to the specified socket. Returns false on error.
static bool nbWrite(int socket, std::string& s, int *bytesSoFar);
static bool nbWrite(int socket, const std::string& s, int *bytesSoFar);


// The next four methods are appropriate for servers.
Expand All @@ -62,7 +62,7 @@ namespace XmlRpc {


//! Connect a socket to a server (from a client)
static bool connect(int socket, std::string& host, int port);
static bool connect(int socket, const std::string& host, int port);


//! Returns last errno
Expand Down
55 changes: 33 additions & 22 deletions utilities/xmlrpcpp/src/XmlRpcSocket.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ XmlRpcSocket::accept(int fd)

// Connect a socket to a server (from a client)
bool
XmlRpcSocket::connect(int fd, std::string& host, int port)
XmlRpcSocket::connect(int fd, const std::string& host, int port)
{
sockaddr_storage ss;
socklen_t ss_len;
Expand All @@ -207,9 +207,13 @@ XmlRpcSocket::connect(int fd, std::string& host, int port)
struct addrinfo hints;
memset(&hints, 0, sizeof(hints));
hints.ai_family = AF_UNSPEC;
if (getaddrinfo(host.c_str(), NULL, &hints, &addr) != 0)
{
fprintf(stderr, "Couldn't find an %s address for [%s]\n", s_use_ipv6_ ? "AF_INET6" : "AF_INET", host.c_str());
int getaddr_err = getaddrinfo(host.c_str(), NULL, &hints, &addr);
if (0 != getaddr_err) {
if(getaddr_err == EAI_SYSTEM) {
XmlRpcUtil::error("Couldn't find an %s address for [%s]: %s\n", s_use_ipv6_ ? "AF_INET6" : "AF_INET", host.c_str(), XmlRpcSocket::getErrorMsg().c_str());
} else {
XmlRpcUtil::error("Couldn't find an %s address for [%s]: %s\n", s_use_ipv6_ ? "AF_INET6" : "AF_INET", host.c_str(), gai_strerror(getaddr_err));
}
return false;
}

Expand Down Expand Up @@ -251,24 +255,31 @@ XmlRpcSocket::connect(int fd, std::string& host, int port)

if (!found)
{
printf("Couldn't find an %s address for [%s]\n", s_use_ipv6_ ? "AF_INET6" : "AF_INET", host.c_str());
XmlRpcUtil::error("Couldn't find an %s address for [%s]\n", s_use_ipv6_ ? "AF_INET6" : "AF_INET", host.c_str());
freeaddrinfo(addr);
return false;
}

// For asynch operation, this will return EWOULDBLOCK (windows) or
// EINPROGRESS (linux) and we just need to wait for the socket to be writable...
int result = ::connect(fd, (sockaddr*)&ss, ss_len);
bool success = true;
if (result != 0 ) {
int error = getError();
if ( (error != EINPROGRESS) && error != EWOULDBLOCK) { // actually, should probably do a platform check here, EWOULDBLOCK on WIN32 and EINPROGRESS otherwise
printf("::connect error = %d\n", getError());
}
int error = getError();
// platform check here, EWOULDBLOCK on WIN32 and EINPROGRESS otherwise
#if defined(_WINDOWS)
if (error != EWOULDBLOCK) {
#else
if (error != EINPROGRESS) {
#endif
XmlRpcUtil::error("::connect error = %s\n", getErrorMsg(error).c_str());
success = false;
}
}

freeaddrinfo(addr);

return result == 0 || nonFatalError();
return success;
}


Expand Down Expand Up @@ -308,7 +319,7 @@ XmlRpcSocket::nbRead(int fd, std::string& s, bool *eof)

// Write text to the specified socket. Returns false on error.
bool
XmlRpcSocket::nbWrite(int fd, std::string& s, int *bytesSoFar)
XmlRpcSocket::nbWrite(int fd, const std::string& s, int *bytesSoFar)
{
int nToWrite = int(s.length()) - *bytesSoFar;
char *sp = const_cast<char*>(s.c_str()) + *bytesSoFar;
Expand Down Expand Up @@ -372,18 +383,18 @@ int XmlRpcSocket::get_port(int socket)
{
sockaddr_storage ss;
socklen_t ss_len = sizeof(ss);
getsockname(socket, (sockaddr *)&ss, &ss_len);
if(getsockname(socket, (sockaddr *)&ss, &ss_len) == 0) {
sockaddr_in *sin = (sockaddr_in *)&ss;
sockaddr_in6 *sin6 = (sockaddr_in6 *)&ss;

sockaddr_in *sin = (sockaddr_in *)&ss;
sockaddr_in6 *sin6 = (sockaddr_in6 *)&ss;

switch (ss.ss_family)
{
case AF_INET:
return ntohs(sin->sin_port);
case AF_INET6:
return ntohs(sin6->sin6_port);
}
switch (ss.ss_family)
{
case AF_INET:
return ntohs(sin->sin_port);
case AF_INET6:
return ntohs(sin6->sin6_port);
}
}
return 0;
}

11 changes: 11 additions & 0 deletions utilities/xmlrpcpp/test/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,17 @@ if(TARGET xmlrpcvalue_base64)
target_link_libraries(xmlrpcvalue_base64 xmlrpcpp)
endif()

catkin_add_gtest(test_socket
test_socket.cpp
test_system_mocks.c
../src/XmlRpcSocket.cpp
../src/XmlRpcUtil.cpp
)
set_target_properties(test_socket PROPERTIES
LINK_FLAGS
"-Wl,--wrap=accept -Wl,--wrap=bind -Wl,--wrap=close -Wl,--wrap=connect -Wl,--wrap=getaddrinfo -Wl,--wrap=getsockname -Wl,--wrap=listen -Wl,--wrap=read -Wl,--wrap=setsockopt -Wl,--wrap=select -Wl,--wrap=socket -Wl,--wrap=write -Wl,--wrap=fcntl -Wl,--wrap=freeaddrinfo"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fancy. TIL.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would expect these arguments to fail on Windows?

)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The block within the if and endif should be indented one level.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


catkin_add_gtest(TestValues TestValues.cpp)
target_link_libraries(TestValues xmlrpcpp)

Expand Down
Loading