1

I have the following code:

#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

#define RCVBUFSIZE 1024

void insertarOperacion(char *op){
        char operacion[RCVBUFSIZE], *retrString = "RETR "; //<
        bool operacionError = false;

        printf("Para solicitar un fichero escriba get <nombreFechero>, para salir escriba quit\n");
        do{
            if(operacionError)
                printf("Opcion invalida\n");
            printf("Operacion: ");
            fgets(operacion, sizeof(operacion), stdin);
            operacion[strlen(operacion) - 1] = '\0';
            operacionError = true; printf("000000000\n");
        } while( ((strcmp(operacion, "quit") == 0) + (strncmp(operacion, "get", 3) == 0)) != 1);
        printf("111111111\n");
        if (strcmp(operacion, "quit") == 0){
            strcpy(operacion, "QUIT\r\n");
            operacion[strlen(operacion) - 1] = '\0';
            strcpy(op, operacion);
        }
        else if(strncmp(operacion, "get",3) == 0){printf("DALEEEKLAJDSFK");
            strcat(retrString, operacion + 4);printf("33333333333\n");
            strcat(retrString, "\r\n"); //>
            retrString[strlen(retrString) - 1] = '\0';
            printf("333333333334\n");
            strcpy(op, retrString);
            printf("333333333335\n");
        } printf("PORFI?");
    //  send(clientSock, retrString, RCVBUFSIZE, 0);
}
int main(){

    //int i = 10, j=20;
    char jiji[RCVBUFSIZE] = "lkajsdfkjdsf";
    insertarOperacion(jiji);
    printf("%s\n\n", jiji);
    insertarOperacion(jiji);
    printf("%s", jiji);



return 0;
}

The problem is that the "quit" section works perfectly. The "get" section does not work. If I input for example: "get rekt.txt". I got the following output:

000000000

111111111

Segmentation fault (core dumped)

Community
  • 1
  • 1
boludo kid
  • 154
  • 1
  • 12
  • Have you tried allocating space for retrString, instead of just assigning a literal? – JGroven Apr 27 '18 at 20:54
  • 2
    `retrsing` is a char pointer that is initialized to point to a static string. Your get section is trying to write into that static area. You need to allocate space for it. – bruceg Apr 27 '18 at 20:55
  • when calling any of the heap allocation functions: malloc calloc realloc, always check (!=NULL) the returned value to assure the operation was successful. If not successful, call perror() with your error message. This function will output both you error message and the text of why the system thinks the error occurred to stderr – user3629249 Apr 28 '18 at 21:27
  • for ease of understanding and readability: 1) separate code blocks for if else while do..while` switch case default via a single blank line – user3629249 Apr 28 '18 at 21:28
  • when calling C library functions, like fgets(), always check the returned value to assure the operation was successful – – user3629249 Apr 28 '18 at 21:28

1 Answers1

2

This:

char *retrString = "RETR ";

is a string literal, which means that the memory this pointer points to is read-only, thus it can not be modified/written.

Your "get" section violates exactly the above, when it tries to modify that string literal:

strcat(retrString, operacion + 4);
strcat(retrString, "\r\n"); //>
retrString[strlen(retrString) - 1] = '\0';

This results to a segmentation fault.

In order to get around this issue, just declare the char pointer, then allocate space for it, populate it and then modify it at will.

Example:

char* retrString = NULL;
retString = malloc(sizeof("RETR ") * sizeof(char) + 1); // +1 for the null terminator
strcpy(retrString, "RETR ");
// do your get thing then
strcat(retrString, operacion + 4);
strcat(retrString, "\r\n"); //>
retrString[strlen(retrString) - 1] = '\0';
gsamaras
  • 71,951
  • 46
  • 188
  • 305
  • the posted code is missing the statement: `#include ` for the function: `strlen()`, `strcpy()` , `strcat()`, etc. – user3629249 Apr 28 '18 at 16:35
  • the expression: `sizeof( char )` is defined in the C standard as `. multiplying anything by 1 has no effect. in the call to `malloc()`, that expression just clutters the code, making it more difficult to understand, debug, etc – user3629249 Apr 28 '18 at 16:38