2

I am writing C++ socket code using Visual Studio Express 2013 in a .dll project. I am at the point where I am getting an error at this sendto function:

/* Send data back */
            if (sendto(sd, (const char *)sendBuffer,(int)sizeof(sendBuffer), 0,
                (struct sockaddr *)&client, sizeof(client)) ==
                SOCKET_ERROR)
            {
                err = WSAGetLastError();
                closesocket(sd);
                WSACleanup(); 
                throw std::runtime_error("Error sending datagram.");


            }

I know that we can get an error code with WSAGetLastError(), but if I initialize it to a variable I just seem to get junk numbers. How do I extract the sendtoerror? I have unit tests written to test this dll, but I'm not sure if there is a way I can just print out the error.

Edit: So after changing the condition to == not != it no longer fails at sendtobut now fails at

bytes_received = recvfrom(sd, readBuffer, NTP_PACKET_MAX, 0, (struct sockaddr *)&client, &len);

            if (bytes_received == SOCKET_ERROR)
            {
                err = WSAGetLastError(); 
                closesocket(sd);
                WSACleanup();
                throw std::runtime_error("Could not receive datagram.");
            }

Like in the original question, i'm unsure of how to get the proper error. When debugging the recvfrom function, I hover over err and it says the value it holds is 114351196. From what I can see, that is not a valid Windows Socket Error Code.

ecain
  • 1,282
  • 5
  • 23
  • 54
  • could you show the code where you get junk numbers? capturing the return value of `sendto()` into a variable seems to be the right way to go. – qdii Dec 07 '15 at 14:43
  • 1
    also, you might want to use proper C++ casts operator: for instance, when casting `const struct sockaddr *` into `struct sockaddr *`, please use `const_cast(&client)` instead of the C-cast, it will help you catch errors early. – qdii Dec 07 '15 at 14:45
  • When I try and use const_cast I get the error "a const_cast can only adjust type modifiers; it cannot change the underlying type". – ecain Dec 07 '15 at 14:51
  • @Elijah What's the value of `len` when you call `recvfrom`? What type is `err`? Where are you placing your breakpoint (or which line of code are you on) when you inspect `err`? – David Schwartz Dec 07 '15 at 15:35
  • @DavidSchwartz `len` is `int len = sizeof(client)`. `err` is a type `int`. I'm placing my breakpoint at `make_packet(&ntpData, NTP_CLIENT, current_value);` which is actually below the `recvfrom` function. After it executes `sendto` it actually goes back up to the `recvfrom` function block. – ecain Dec 07 '15 at 15:47
  • @Elijah What type is `err`? – Dennis Dec 07 '15 at 16:33
  • @Dennis it's an `int` – ecain Dec 07 '15 at 16:38
  • @Elijah When you hover over the value the highlighted line should be the line after the `err = WSAGetLastError();`. Otherwise you are checking the value before it has been assigned. To be certain I would keep the err initialisation on the same line as the assignment. See update in answer. – Dennis Dec 07 '15 at 16:40

1 Answers1

1

Couple of bad mistakes in your code

        /* Send data back */
        if (sendto(sd, (const char *)sendBuffer,(int)sizeof(sendBuffer), 0,
            (struct sockaddr *)&client, sizeof(client)) !=
            SOCKET_ERROR) // SHOULD BE == (1)
        {
            throw std::runtime_error("Error sending datagram.");
            // UNREACHABLE CODE (2)
            closesocket(sd);
            WSACleanup();
        }

Error 1: Your error block is entered when you are successfully sending. This may mean that your error is uninitialised (it will be the last error value from somewhere else if not). According to the MSDN documentation for WSAGetLastError, the function should be called immediately after your WinSocket operation fails, and only in the those circumstances.

If you call it after a successful operation (which you are doing) then there is no guarantee of what value you will receive. If could be the last error from another WinSocket operation on this thread, it may be 0, it may be some other value. The docs do not state what happens if the error is retrieved when no operations have failed, and so you can think of this as undefined behaviour.

Error 2: After you throw your exception the function will exit immediately. You will not call closesocket or WSACleanup

UPDATE:

I'm guessing that you are checking the value of err in the debugger before you have assigned it any value. You should be doing something along the lines of:

bytes_received = recvfrom(sd, readBuffer, NTP_PACKET_MAX, 0, (struct sockaddr *)&client, &len);
if (bytes_received == SOCKET_ERROR)
{
    int err = WSAGetLastError(); 
    // In debugger you must be here before err is set
    closesocket(sd);
    WSACleanup();
    std::string what{"Could not receive datagram."};
    what+= std::to_string(err);
    throw std::runtime_error(what.c_str());
}
Dennis
  • 3,683
  • 1
  • 21
  • 43