CODEWITHSUNDEEP
cstringloops
mrfr
mrfr

Reputation: 1794

Why do i get an Segmentation fault?

I know it has to do with my for loop. Have tried to modify it, but whatever I put in for the arguments, I still get an Segmentation fault.

#include <ctype.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int
main(int argc, char * argv[])
{
   char* request_target = "/cat.html?q=Alice hej";

   // TODO: extract query from request-target
   char query[20];

   // finds start of query
   char* startofquery = strstr(request_target, "q=");
   if (startofquery != NULL)
   {
       int j = 0;
       for (int i = strlen(request_target) - strlen(startofquery); i  == ' '; i++, j++)
       {
           query[j] = request_target[i];
       }
       request_target[j] = '\0';
   }
   else
   {
       printf("400 Bad Request");
   }

   printf("%s", query);
}

Upvotes: 1

Views: 88

Answers (3)

Sourav Ghosh
Sourav Ghosh

Reputation: 134286

There are two points that I can see appears to be troublesome.

  1. in your for loop, the terminating condition

    i  == ' '

    should be

    request_target[i] != ' '

    if you want to run the loop until it gets a space character, or, 

    request_target[i] != '\0'

    if you would want to run the loop til the end of the source string.

  2. request_target[j] = '\0'; should be query[j] = '\0'; because,

    1) You cannot modify a string literal, it's undefined behavior.

    2) Probably you want to null terminate the destination array.

Upvotes: 0

Manos Nikolaidis
Manos Nikolaidis

Reputation: 22224

This line defines a string literal 

char* request_target = "/cat.html?q=Alice hej";

It is undefined behaviour to write to a string literal
you are doing that here :

request_target[j] = '\0';

use a char array instead

char request_target[] = "/cat.html?q=Alice hej";

Also, if I understand correctly you are trying to extract q=Alice from /cat.html?q=Alice hej. The for loop you implemented has a few issues (i == ' ') as mentioned in other answers. And is not actually necessary. You can copy this part quite simply:

char *startofquery = strstr(request_target, "q=");
char *endofquery = strchr(startofquery, ' ');
int querySize = endofquery - startofquery;
if (startofquery != NULL && endofquery != NULL) {
    memcpy(query, startofquery, querySize);
    query[querySize] = '\0';
}

This is less error prone and will most likely perform better. In this case you don't need to define request_target as an array but I would recommend to make it const so that you will get a useful compiler error if you attempt to write:

const char *request_target = "/cat.html?q=Alice hej";

Upvotes: 4

Mr. E
Mr. E

Reputation: 2120

I see two problems in your for loop. The first one is the condition:

 for (int i = strlen(request_target) - strlen(startofquery); i  == ' '; i++, j++)

Here i is an integer (position in your string), but you are comparing against a char (' '). It should be:

for (int i = strlen(request_target) - strlen(startofquery); request_target[i]  != ' '; i++, j++)

The second problem is you're trying to write into a string literal (UB) here:

 request_target[j] = '\0';

It should be:

 query[j] = '\0';

Upvotes: 0

Related Questions