For some background, I am writing a TCP socket server / client communication program to copy one text file to another. I'm struggling with my implementation of reading over the socket. I'm developing on linux.
This is the server-side read function that is called when the client side read function sends the read opcode through the socket. The program reaches this code properly with the correct fd and count.
/* s_read
* server side read
* Send in socket: 4 bytes return value, 4 bytes errno, n bytes data
*/
int s_read(int conn){
// Get fd
unsigned char fdstr[4];
int L = sizeof(char) * 4;
int bytes = 0;
int total = 0;
while ( total < L ) {
if ( (bytes = read(conn, fdstr total, L - total)) < 0) {
printf("r_server.c::s_read: Couldn't receive fd\n");
return -1;
}
total = bytes;
}
int fd = 0;
fd = (fdstr[0] << 24) | (fdstr[1] << 16) | (fdstr[2] << 8) | fdstr[3];
// Get count
unsigned char countstr[4];
L = sizeof(char) * 4;
bytes = 0;
total = 0;
while ( total < L ) {
if ( (bytes = read(conn, countstr total, L - total)) < 0) {
printf("r_server.c::s_read: Couldn't receive count\n");
return -1;
}
total = bytes;
}
int count = 0;
count = (countstr[0] << 24) | (countstr[1] << 16) | (countstr[2] << 8) | countstr[3];
// Prepare return message
int return_value;
L = 8 // 4 bytes for return value, 4 bytes for errno
count; // count bytes for read in data
int error = 0;
char * msg;
char * read_value = malloc(count);
// Execute open call
bytes = 0;
total = 0;
while ( total < count ) {
if ( (return_value = read(fd, read_value total, count - total)) < 0 ) {
error = errno;
break;
}
total = return_value;
}
// Build return message
msg = malloc(L);
L=0;
msg[L ] = (return_value >> 24) & 0xff; // put the kernel return value
msg[L ] = (return_value >> 16) & 0xff;
msg[L ] = (return_value >> 8) & 0xff;
msg[L ] = (return_value ) & 0xff;
msg[L ] = (error >> 24) & 0xff; // put the errno
msg[L ] = (error >> 16) & 0xff;
msg[L ] = (error >> 8) & 0xff;
msg[L ] = (error ) & 0xff;
for (int i=0; i < count; i )
msg[L ] = read_value[i]; // put the read in data.
// Send return message
bytes = 0;
total = 0;
while ( total < L ) {
if ( (bytes = write(conn, msg total, L - total)) < 0) {
printf("r_server.c::s_read: Error sending r_read return value to client\n");
return -1;
}
total = bytes;
}
free(read_value);
free(msg);
return 0;
}
This is the client-side read function that puts together a payload and sends it over the socket to request the server to read on its end.
/* r_read
* remote read
*/
int r_read(int fd, void *buf, int count) {
int L;
char * msg;
int in_msg;
int in_err;
L = 1 // byte for opcode
sizeof(fd) // int bytes for fd.
sizeof(count); // int bytes for count.
msg = malloc(L);
L=0;
msg[L ] = 3; // this is the code for read.
msg[L ] = (fd >> 24) & 0xff; // put the fd.
msg[L ] = (fd >> 16) & 0xff;
msg[L ] = (fd >> 8) & 0xff;
msg[L ] = (fd ) & 0xff;
msg[L ] = (count >> 24) & 0xff; // put the count.
msg[L ] = (count >> 16) & 0xff;
msg[L ] = (count >> 8) & 0xff;
msg[L ] = (count ) & 0xff;
int bytes = 0;
int total = 0;
while ( total < L ) {
if ( (bytes = write(sock, msg total, L - total)) < 0) {
printf("Failed to send r_read to server\n");
return -1;
}
total = bytes;
}
bytes = 0;
total = 0;
while ( total < 8 ) {
if ( (bytes = read(sock, msg total, 8 - total)) < 0) {
printf("Failed to receive r_read from server\n");
return -1;
}
total = bytes;
}
in_msg = (msg[0] << 24) | (msg[1] << 16) | (msg[2] << 8) | msg[3];
in_err = (msg[4] << 24) | (msg[5] << 16) | (msg[6] << 8) | msg[7];
for (int i = 0; i < count; i ) {
*(char *)(buf i) = msg[i 8];
}
errno = in_err;
free(msg);
return in_msg;
}
The client is receiving a bogus return_value in in_msg, but the server sees the proper return_value before sending it. The actual read in data is also bogus seemingly on both ends.
This is the code actually calling the i/o functions to copy the one file to the other. I have a similar one that copies from a local file to a remote file, and that one works properly. It is called from main after main receives the socket information and strips those args from argv[].
int entry(int argc, char* argv[]){
// Input guards
if (argc != 3) {
printf("Invalid arguments\n");
return -1;
}
// Get file names
char *filename = argv[1];
char *copyfilename = argv[2];
printf("rclient2.c::entry: Copying remote file %s to local file %s\n", filename, copyfilename);
// Open files
int fd = r_open((const char*) filename, O_RDWR, (mode_t) 0600);
if (fd < 0) {
printf("rclient2.c::entry: r_open failed.\n");
return -1;
}
int copyfd = open((const char*) copyfilename, O_RDWR | O_CREAT, (mode_t) 0600);
if (copyfd < 0) {
printf("rclient2.c::entry: open failed.\n");
return -1;
}
// Seek to position 10
//r_lseek(fd, 10, SEEK_SET); // Later requirement once read is working
// Copy file
char buf;
while ( r_read(fd, &buf, 1) > 0 ) {
if (write(copyfd, &buf, 1) < 0) {
printf("rclient2::entry: write failed.\n");
return -1;
}
}
// Close files
r_close(fd);
close(copyfd);
return 0;
}
More detail on the incorrect behavior: The server is only getting a s_read() call 6 times before closing in a case where it should be getting about 41 of them. Upon reading the "copied" file, it is full of bogus values that are not human readable. None of my errors are getting tripped except for one after the close() call. Specifically the "r_server.c::main: Couldn't receive opcode." error. Because of this, I'm including the server's main function below (where it reads the opcode provided by the client functions).
/* main - server implementation
*/
int main(int argc, char *argv[]){
// Setup socket data
int listener, conn;
unsigned int length;
struct sockaddr_in s1, s2;
// Create server socket
listener = socket(AF_INET, SOCK_STREAM, 0);
if (listener < 0) {
printf("r_server.c::main: Error creating server socket\n");
return -1;
}
// Establish server socket
bzero((char *) &s1, sizeof(s1));
s1.sin_family = AF_INET;
s1.sin_port = htons(0);
s1.sin_addr.s_addr = inet_addr("127.0.0.1");
if( bind( listener, (struct sockaddr *) &s1, sizeof(s1)) < 0) {
printf("r_server.c::main: Server couldn't bind to the port\n");
return -1;
}
// Print port and start server
length = sizeof(s1);
getsockname(listener, (struct sockaddr *) &s1, &length);
printf("%d\n", s1.sin_port);
if ( listen(listener, 1) < 0) {
printf("r_server.c::main: Server error while listening\n");
return -1;
}
// While server running
while(1) {
// Connect to new client
length = sizeof(s2);
conn = accept(listener, (struct sockaddr *) &s2, &length);
if( conn < 0 ) {
printf("r_server.c::main: Server failed to accept incoming stuff\n");
return -1;
}
// Fork to manage client and listen for new clients
if (fork() == 0) {
// Until client disconnects
while (1) {
// Get opcode
unsigned char opcode;
int success;
if ((success = read(conn, &opcode, sizeof(opcode))) < 0) {
printf("r_server.c::main: Couldn't receive opcode. opcode = %d\n", opcode);
return -1;
}
// Client disconnected
if (success == 0) {
return 0;
}
// Call related server side function
switch (opcode) {
case (1):
printf("Opening...\n");
if (s_open(conn) < 0)
return -1;
break;
case (2):
printf("Closing...\n");
if (s_close(conn) < 0)
return -1;
break;
case (3):
printf("Reading...\n");
if (s_read(conn) < 0)
return -1;
break;
case (4):
printf("Writing...\n");
if (s_write(conn) < 0)
return -1;
break;
case (5):
printf("Seeking...\n");
if (s_lseek(conn) < 0)
return -1;
break;
case (6):
printf("Piping...\n");
if (s_pipe(conn) < 0)
return -1;
break;
case (7):
printf("Duping...\n");
if (s_dup2(conn) < 0)
return -1;
break;
default:
return -1;
}
}
return 0;
}
}
return 0;
}
CodePudding user response：
The code is blindly assuming that read will always read exactly the number of bytes asked for and write will always write the full buffer given. Both of these assumptions are wrong.
read will only read up to the given size but might actually read less. Similar write will only write at most the full buffer, but might write less. Therefore the return value has to be checked to find out how much data are actually read or written. If not everything was read or written the operations have to be repeated for the remaining data, maybe even multiple times.
CodePudding user response：
In the r_read() function, we never read in the value the server put into the response. We're only reading in the 8 bytes corresponding to the return_value and errno.
Instead of
int bytes = 0;
int total = 0;
while ( total < L ) {
if ( (bytes = write(sock, msg total, L - total)) < 0) {
printf("Failed to send r_read to server\n");
return -1;
}
total = bytes;
}
bytes = 0;
total = 0;
while ( total < 8 ) {
if ( (bytes = read(sock, msg total, 8 - total)) < 0) {
printf("Failed to receive r_read from server\n");
return -1;
}
total = bytes;
}
in_msg = (msg[0] << 24) | (msg[1] << 16) | (msg[2] << 8) | msg[3];
in_err = (msg[4] << 24) | (msg[5] << 16) | (msg[6] << 8) | msg[7];
for (int i = 0; i < count; i ) {
*(char *)(buf i) = msg[i 8];
}
We need to do
int bytes = 0;
int total = 0;
while ( total < L ) {
if ( (bytes = write(sock, msg total, L - total)) < 0) {
printf("Failed to send r_read to server\n");
return -1;
}
total = bytes;
}
bytes = 0;
total = 0;
while ( total < 8 ) {
if ( (bytes = read(sock, msg total, 8 - total)) < 0) {
printf("Failed to receive r_read from server\n");
return -1;
}
total = bytes;
}
in_msg = (msg[0] << 24) | (msg[1] << 16) | (msg[2] << 8) | msg[3]; // # of bytes remaining in the socket buffer (the data we asked for)
in_err = (msg[4] << 24) | (msg[5] << 16) | (msg[6] << 8) | msg[7];
bytes = 0;
total = 0;
while ( total < in_msg ) {
if ( (bytes = read(sock, buf total, in_msg - total)) < 0) {
printf("Failed to read the r_read data from server\n");
return -1;
}
}
Note that the while ( total ...) logic is only required if you're needing to handle a case where a read or write call does not fully succeed. In my case, we're allowed to simply exit with an error if this happens.