I am creating a TCP Client that can send messages to the server (using winsock2) and in turn the server sends the message to every other socket, but for some reason it recv() isn't getting the data of it's own message.
#include <stdio.h>
#include <windows.h>
#include <winsock2.h>
#include <time.h>
#include <string.h>
#pragma comment(lib, "ws2_32.lib")
DWORD WINAPI receive(LPVOID s) {
char output[4096];
for(;;) {
recv((SOCKET) s, output, sizeof(output), 0);
printf("%s", output);
}
return 0;
}
int main() {
WSADATA wsa;
SOCKET s;
struct sockaddr_in server;
clock_t start;
double finished;
start = clock();
printf("initializing...\n");
if(WSAStartup(MAKEWORD(2, 2), &wsa) != 0) {
printf("failed: %d", WSAGetLastError());
//sleep(3);
return 1;
}
finished = (double) (clock() - start) / CLOCKS_PER_SEC;
printf("initialized %fs\n", finished);
start = clock();
printf("creating socket...\n");
if((s = socket(AF_INET, SOCK_STREAM, 0)) == INVALID_SOCKET) {
printf("socket creation failed: %d", WSAGetLastError());
//sleep(3);
return 1;
}
finished = (double) (clock() - start) / CLOCKS_PER_SEC;
printf("socket created %fs\n", finished);
printf("what ip would you like to connect to: ");
char ip[64];
scanf("%s", ip);
server.sin_addr.s_addr = inet_addr(ip);
server.sin_family = AF_INET;
server.sin_port = htons(8080);
if(connect(s, (struct sockaddr*) &server, sizeof(server)) < 0) {
printf("connect error\n");
return 1;
}
printf("connection to %s was successful\n", ip);
char username[256];
printf("what username would you like to use: ");
scanf(" %[^\n]s", &username);
HANDLE handle = CreateThread(NULL, 0, receive, &s, 0, NULL);
char input[4096];
char buff[4096];
for(;;) {
printf(">: ");
scanf(" %[^\n]s", &input);
sprintf(buff, "%s> %s", username, input);
send(s, buff, sizeof(buff), 0);
}
return 0;
}
The server was programmed in python, although I do not think that is the issue considering it works when I have the recv() right after the send().
CodePudding user response:
You are passing a pointer to a SOCKET s
to your thread worker function:
SOCKET s;
// ...
HANDLE handle = CreateThread(NULL, 0, receive, &s, 0, NULL);
That function is treating the passed argument as if it were a SOCKET:
DWORD WINAPI receive(LPVOID s) {
char output[4096];
for(;;) {
recv((SOCKET) s, output, sizeof(output), 0);
printf("%s", output);
}
return 0;
}
So basically you're not reading from the correct file descriptor but some random address.
Dereferencing the pointer inside the worker function should solve that issue:
DWORD WINAPI receive(LPVOID arg) {
SOCKET *s = arg;
char output[4096];
for(;;) {
recv(*s, output, sizeof(output), 0);
printf("%s", output);
}
return 0;
}
Not checking the return value of recv
and using printf
as if output
were guaranteed to be a null-terminated string is also a serious issue. Therefore, I recommend the following code instead:
DWORD WINAPI receive(LPVOID arg) {
SOCKET *s = arg;
char output[4096];
int len;
for(;;) {
len = recv(*s, output, sizeof(output), 0);
if (len > 0)
printf("%.*s", len, output);
}
return 0;
}
That way, output
does not have to be null-terminated.