I'm doing an exercise about UDP sockets in C. When the client sends a specific message (e.g hi) the server has to send "Nice to meet you". If no standard reply is found the server sends "No suitable reply". My problem is that memset fails if I try to return the reply like this:
return "No suitable reply";
and it doesn't if I return the reply in this way:
char* foo = malloc(sizeof(char*));
memset(foo, 0, strlen(ses));
memcpy(foo, "No suitable reply", 17);
return foo;
I tried to google a solution to this and I found this and this, but they don't seem to address my problem (I first thought that memset doesn't work on a string declared like char string[] = "something" but in the second example they use memset on a static string).
Here is the whole code (the memset I'm talking about is right at the end):
/*
Alessandro Dussin 5AI
2018-17-11
Write a program to handle a single UDP "connection"
*/
//Standard libraries
#include <stdio.h>
#include <stdlib.h>
//Sockets libraries and connection ahndling
#include <sys/socket.h>
#include <netinet/in.h>
#include <sys/types.h>
#include <arpa/inet.h>
//Read/write ops on file descriptors
#include <unistd.h>
//String ops
#include <string.h>
#include <assert.h>
void chopN(char *str, size_t n)
{
assert(n != 0 && str != 0);
size_t len = strlen(str);
if (n > len)
return; // Or: n = len;
memmove(str, str+n, len - n + 1);
}
//Required by the exercise. Given a certain word or phrase, reply with a specific string
char* switchreply(char* str){
//Extracts the word or phrase (Basically removes the "/command " word)
chopN(str, strlen("/stdreply "));
int i = 0;
for(; i < strlen(str); i++){
if(str[i] == '\n'){
str[i] = '\0';
break;
}
}
if(strcmp(str, "ciao") == 0){
return "ciao anche a te!";
}
else if(strcmp(str, "I hate you") == 0){
return "I hate you too!";
}
return "";
}
char* stdreply(char *str){
char* tmp = malloc(sizeof(char)*128);
int i = 0;
//printf("Entered stdreply... str at the start of the func: %s\n", str);
for(; i < strlen(str); i++){
tmp[i] = str[i];
//printf("tmp: %s\n", tmp); //DEBUG
if(strcmp(tmp, "/echo ") == 0){ // if(strcmp() == 0) is necessary because
//otherwise 0 would be interpreted as FALSE
//printf("Echo detected\n"); //DEBUG
chopN(str, strlen("/echo "));
str[strlen(str)] = '\0';
return str;
}
else if(strcmp(tmp, "/stdreply ") == 0){
//printf("I got into the else if\n"); //DEBUG
char* tmpreply = calloc(strlen(str), sizeof(char*));
tmpreply = switchreply(str);
//printf("tmpreply: %s\n", tmpreply);
str = malloc(sizeof(char*)*strlen(tmpreply));
memcpy(str, tmpreply, strlen(tmpreply));
//str[strlen(str)] = '\0'; //DEBUG
//printf("str: %s\n", str); //DEBUG
return str;
}
else if(strcmp(tmp, "/TODO") == 0){
char* ses = malloc(sizeof(char*));
memset(ses, 0, strlen(ses));
memcpy(ses, "work in progress", 17);
return ses;
}
}
return "No suitable reply";
}
int main(int argc, char **argv){
if(argc < 2){
printf("Usage: ./server port");
exit(0);
}
int serverfd;
serverfd = socket(AF_INET, SOCK_DGRAM, 0);
struct sockaddr_in server;
server.sin_port = htons(atoi(argv[1]));
server.sin_family = AF_INET;
server.sin_addr.s_addr = INADDR_ANY;
if(bind(serverfd, (struct sockaddr *)&server, sizeof(server)) < 0){
perror("Bind() error: ");
fflush(stderr);
}
//"UDP message receiver" variables declarations
int bytes; //Reads how many bytes the funcion recvfrom has read
struct sockaddr_in from;
char* buffer = malloc(sizeof(char*)); //String to which save the client message
memset(buffer, 0, strlen(buffer)); //and set it to zero
socklen_t fromlen = sizeof(struct sockaddr_in);
const char stdrep[] = "Message Received: "; //This string will always be
//printed upon receiving a message
char* reply = malloc(sizeof(char*)); //This is where the return value of
//stdreply() will be stored
memset(reply, 0, strlen(reply)); //and set it zero
//This while will keep "listening" for udp messages
while((bytes = recvfrom(serverfd, buffer, 1024, 0, (struct sockaddr *)&from, &fromlen)) > 0){
//From teacher's example. Write to stdout
write(1, stdrep, strlen(stdrep));
write(1, buffer, bytes);
//Detect a basically empty string (if the client has pressed only enter)
if(buffer[0] == '\n'){
bytes = sendto(serverfd, "You pressed only enter!\n", 18, 0, (struct sockaddr *)&from, fromlen);
}
//Act according to the client message
reply = stdreply(buffer);
bytes = sendto(serverfd, reply, strlen(reply), 0, (struct sockaddr *)&from, fromlen);
if (bytes < 0){
perror("sendto: ");
fflush(stderr);
}
memset(buffer, 0, 1024);
memset(reply, 0, strlen(reply)); //The seg fault happens right here
fflush(stdout);
}
return 0;
}
There are plenty of problems in the code you are posting.
As @JonBolinger already noted, sizeof(char*) returns the size in bytes of a pointer to a char. On Intel platforms this will be either 4 or 8, depending on whether you are running on 32-bit or 64-bit. (So you end up allocating buffers of 4 or 8 bytes)
You consistently try to clear your dynamically-allocated buffers with memset(). malloc() will return memory filled with garbage, and you indicate how many bytes to clear by using strlen() on the returned buffer. strlen() will scan the buffer until it finds the first 0 character to compute the string's length. Since the buffer is filled with garbage, this can easily give you a value outside of the boundaries of your memory block and you will end up corrupting memory.
Every call to malloc() should be matched to a free() call, otherwise you will leak memory. This is especially important if your program is long-running.
When you are working with temporary local strings (strings that are not returned to the caller), it is very common practice to use a local char array instead of malloc(). This way, the buffer gets allocated on the stack and will be released automatically when your function exits scope. Just be sure to use 'safe' string functions like strncpy() that will accept as a parameter the length of the buffer, to avoid overwrites.
void Example(char* anotherString ) {
char tmpString[256]; // this will create a local buffer with capacity of 256 bytes
strncpy(tmpString, anotherString, sizeof(tmpString)); // copy string, without risk of overflowing the buffer
}
Warning: NEVER attempt to return a local temporary buffer as a result, remember that it will no longer exist when the function exits and although the returned value may initially have meaningful results, they will certainly be destroyed as soon as you call another function. Instead of this, another common practice when you need a string return value, instead of returning a string allocated with malloc() -that would need to be released with free() - you pass a local buffer that will hold the result as a parameter, like this:
void func1() {
char result[256];
func2(result, 256);
// after calling, result will carry "a returned string"
}
void func2(char* result, size_t bufferLen) {
strncpy(result, "a returned string", bufferLen);
}
I think your code would benefit greatly if you can transform it to using this style where applicable.