1

I am working on a UDP socket in C++ that is a client and also a server at one time. I am using Visual Studio 2017 and run it on Windows 7. I have some troubles with recvfrom() function, that is ending with "invalid argument" error. I think socket is okay, sockaddr_in structs are okay, thread is okay and even the while-loop with sendto() function is okay. But the thread ends after calling a recvfrom() function and I really don't know where the problem is...

#include <iostream>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <winsock2.h>
#include <thread>

#pragma comment(lib, "Ws2_32.lib")

using namespace std;

void printMessage(SOCKET s, SOCKADDR_IN destAddr) {

    char message[100];
    int result, error;
    int addrSize = sizeof(destAddr);
    do {
        printf("som v threade\n");
        result = recvfrom(s, message, 100, 0, (SOCKADDR*)&destAddr, &addrSize);
        printf("%d\n", result);

        if (result < 0) {
            error = WSAGetLastError();
            printf("%d\n", error);
        }

        if (result > 0)
            printf("%s", message);

    } while (result > 0);

}


int main(int argc, char *argv[]) {

    WORD dllVersion = MAKEWORD(2, 1);
    WSADATA wsaData;

    if (WSAStartup(dllVersion, &wsaData) != 0) {
        printf("Winsock startup failed");
        exit(1);
    }

    const char *ip = "127.0.0.1";
    int dstPort = 49999;
    int srcPort = 49999;

    SOCKADDR_IN destaddr, srcaddr;
    SOCKET s;
    s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);

    if (s < 0) {
        printf("Error creating socket");
        exit(1);
    }

    destaddr.sin_family = AF_INET;
    destaddr.sin_addr.s_addr = inet_addr(ip);
    destaddr.sin_port = htons(dstPort);

    srcaddr.sin_family = AF_INET;
    srcaddr.sin_addr.s_addr = htonl(INADDR_ANY);
    srcaddr.sin_port = htons(srcPort);


    bind(s, (SOCKADDR*)&srcaddr, sizeof(srcaddr));

    thread receiving (printMessage, s, destaddr);
    receiving.detach();

    while (1) {
        char message[100];

        printf("Write a message:");
        scanf("%s", message);

        if (sendto(s, message, sizeof(message), 0, (SOCKADDR *) &destaddr, sizeof(destaddr)) < 0) {
            printf("Message not sent");
            exit(1);
        }
    }
    return 0;
}
Matus Zelenak
  • 13
  • 1
  • 3
  • You aren't error-checking `bind()`. You should call `WSAGetLastError()` *before* calling `printf()`, or any other system calls, and you should reinitialize `addrSize` every time around the loop. – user207421 Nov 20 '17 at 02:22
  • And looping until the received count is zero is futile unless you know that (a) the peer is going to send a zero-length packet to terminate the sending and (b) it hasn't been reordered ahead of a datagram, which you can't know. – user207421 Aug 19 '19 at 07:54

2 Answers2

2

Error code 10022 is WSAEINVAL. Per the recvfrom() documentation:

WSAEINVAL

The socket has not been bound with bind(), or an unknown flag was specified, or MSG_OOB was specified for a socket with SO_OOBINLINE enabled, or (for byte stream-style sockets only) len was zero or negative.

You are not doing any error handling to make sure your bind() call is not failing. Or even that you are calling the correct bind() function to begin with. WinSock and STL both have their own bind() functions, but your use of using namespace std masks that fact.

Also, you need to reset addrSize each time you call recvfrom(), as it could modify the value. It expects you to pass in your max buffer size, and then it outputs the size actually written to the buffer.

Also, you are not calling WSAGetLastError() at the right time. You need to call it immediately after a failed WinSock call, before doing anything else. In other words, don't call printf() before calling WSAGetLastError(), as it might potentially reset the error code.

Also, if recvfrom() reads successfully, you are assuming the data is null terminated, but that is not guaranteed. You need to pass result as a parameter to printf().

And, you really should not be mixing C style I/O with C++. Use C++ style I/O instead.

Try something more like this:

#include <iostream>
#include <string>
#include <thread>
#include <winsock2.h>

#pragma comment(lib, "Ws2_32.lib")

void printMessage(SOCKET s)
{
    char message[100];
    int result, error, addrSize;
    SOCKADDR_IN clientaddr;

    do {
        std::cout << "som v threade" << std::endl;
        addrSize = sizeof(clientaddr);
        result = recvfrom(s, message, 100, 0, (SOCKADDR*)&clientaddr, &addrSize);
        if (result == SOCKET_ERROR) {
            error = WSAGetLastError();
            std::cout << "Receive failed. Error: " << error << std::endl;
            continue;
        }
        std::cout << "Received " << result << " byte(s) from " << inet_ntoa(clientaddr.sin_addr) << ":" << ntohs(clientaddr.sin_port) << std::endl;
        if (result > 0) {
            std::cout.write(message, result);
            std::cout << std::endl;
        }
    }
    while (result > 0);
}

int main(int argc, char *argv[]) {
    WORD dllVersion = MAKEWORD(2, 1);
    WSADATA wsaData;    
    int result, error;

    result  = WSAStartup(dllVersion, &wsaData);
    if (result != 0) {
        std::cout << "Winsock startup failed. Error: " << result << std::endl;
        exit(1);
    }

    SOCKET s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
    if (s == INVALID_SOCKET) {
        error = WSAGetLastError();
        std::cout << "Socket creation failed. Error: " << error << std::endl;
        exit(1);
    }

    const char *ip = "127.0.0.1";
    unsigned short dstPort = 49999;
    unsigned short srcPort = 49999;

    SOCKADDR_IN destaddr = {};
    SOCKADDR_IN srcaddr = {};

    destaddr.sin_family = AF_INET;
    destaddr.sin_addr.s_addr = inet_addr(ip);
    destaddr.sin_port = htons(dstPort);

    srcaddr.sin_family = AF_INET;
    srcaddr.sin_addr.s_addr = INADDR_ANY;
    srcaddr.sin_port = htons(srcPort);

    result = ::bind(s, (SOCKADDR*)&srcaddr, sizeof(srcaddr));
    if (result == SOCKET_ERROR) {
        error = WSAGetLastError();
        std::cout << "Socket bind failed. Error: " << error << std::endl;
        exit(1);
    }

    std::thread receiving(printMessage, s);
    receiving.detach();

    std::string message;
    do {
        std::cout << "Write a message: ";
        std::getline(std::cin, message);
        result = sendto(s, message.c_str(), message.size(), 0, (SOCKADDR *) &destaddr, sizeof(destaddr));
        if (result == SOCKET_ERROR) {
            error = WSAGetLastError();
            std::cout << "Message not sent. Error: " << error << std::endl;
            exit(1);
        }
    }
    while (true);

    return 0;
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
1

had same problem with me after including files like

#include <thread> /*or*/ #include <future>

turns out it was a confusion between bind() and std::bind().

remove using namespace std; or simply use ::bind().

user207421
  • 305,947
  • 44
  • 307
  • 483
Alishan
  • 19
  • 3