server.c segmentation fault

Question

I couldn't find the cause of segmentation fault when i run the server

I have used valgrind to find where is the memory leaks .. and this is the result :

==2186== Memcheck, a memory error detector
==2186== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==2186== Using Valgrind-3.10.0.SVN and LibVEX; rerun with -h for copyright info
==2186== Command: ./server public
==2186== 
Using /home/ubuntu/workspace/pset6/public for server's root
Listening on port 8080
GET / HTTP/1.1
==2186== Use of uninitialised value of size 8
==2186==    at 0x4C2E1E0: strcpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2186==    by 0x402540: parse (server.c:693)
==2186==    by 0x4018CA: main (server.c:186)
==2186== 
==2186== Invalid write of size 1
==2186==    at 0x4C2E1E0: strcpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2186==    by 0x402540: parse (server.c:693)
==2186==    by 0x4018CA: main (server.c:186)
==2186==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==2186== 
==2186== 
==2186== Process terminating with default action of signal 11 (SIGSEGV)
==2186==  Access not within mapped region at address 0x0
==2186==    at 0x4C2E1E0: strcpy (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==2186==    by 0x402540: parse (server.c:693)
==2186==    by 0x4018CA: main (server.c:186)
==2186==  If you believe this happened as a result of a stack
==2186==  overflow in your program's main thread (unlikely but
==2186==  possible), you can try to increase the size of the
==2186==  main thread stack using the --main-stacksize= flag.
==2186==  The main thread stack size used in this run was 8388608.
==2186== 
==2186== HEAP SUMMARY:
==2186==     in use at exit: 5,682 bytes in 3 blocks
==2186==   total heap usage: 7 allocs, 4 frees, 10,328 bytes allocated
==2186== 
==2186== LEAK SUMMARY:
==2186==    definitely lost: 0 bytes in 0 blocks
==2186==    indirectly lost: 0 bytes in 0 blocks
==2186==      possibly lost: 0 bytes in 0 blocks
==2186==    still reachable: 5,682 bytes in 3 blocks
==2186==         suppressed: 0 bytes in 0 blocks
==2186== Reachable blocks (those to which a pointer was found) are not shown.
==2186== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==2186== 
==2186== For counts of detected and suppressed errors, rerun with: -v
==2186== Use --track-origins=yes to see where uninitialised values come from
==2186== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
Segmentation fault

I think the problem is in my parse function .. but i didn't find the problem

bool parse(const char* line, char* abs_path, char* query)
{
     //allocate memory for copy of line
    char** LineCopy = malloc((sizeof(char) * strlen(line)) + 1);

    //copy line into new variable
    strcpy(*LineCopy, line);

    //allocate memory for method and copy actual method into it
    char* method = malloc(sizeof(char*));
    method = strsep(LineCopy, " ");

    //if method is not get, respond to browser with error 405 and return false
    if (strcasecmp(method, "GET") != 0) {
        error(405);
        return false;
    }

    //put request-target in abs_path
    abs_path = strsep(LineCopy, "]" + 1);

    //if request-target does not begin with /, respond with 501 and return false
    if (abs_path[0] != '/') {
        error(501);
        return false;
    }

    char* HTTP_version = malloc(sizeof(char*));
    HTTP_version = strsep(LineCopy, "\\");
    if(strcasecmp(HTTP_version, "HTTP/1.1") != 0) {
        error(505);
        return false;
    }

    //if request-target contains a "", respond with error 400 and return false
    char* c = abs_path;
    if (strchr("\"", *c)) {
        error(400);
        return false;
    }

    //allocate memory for copy of abs_path
    char** abs_path_COPY = malloc(sizeof(char**));
    //copy line into new variable
    strcpy(*abs_path_COPY, abs_path);
    for (int i = 0, n = strlen(*abs_path_COPY); i < n; i++) {
        for (int j = 0, m = strlen(*abs_path_COPY) - i; j < m; j++) {
            if (*abs_path_COPY[i] == '/' && *abs_path_COPY[i+1] == '?') query[j] = *abs_path_COPY[i+2];
            if (*abs_path_COPY[i] == '/' && *abs_path_COPY[i+1] != '?') query[j] = *abs_path_COPY[i+1];
            // if (strlen(query)) < 1) query = "";
            if (query[j] == '\0') break;
        }
    }

    free(HTTP_version);
    free(c);
    free(method);
    free(LineCopy);
    free(abs_path_COPY);

    return false;
}

Help please :)

Answer 1

Your first order of business is finding out where it seg faults. valgrind is telling you it does, now you need gdb to tell you where.

To get started with gdb:

$ gdb ./server
(gdb) break parse
(gdb) run public
Send a request with browser or curl.

Then you can step through parse to narrow down where the seg fault is coming.

Your approach is solid, build "things", compare to other "things", return results as appropriate. You need to work on the build "things" code.

Some big issues you'll need to deal with:

1. char** LineCopy = malloc((sizeof(char) * strlen(line)) + 1); declaring LineCopy as a pointer-to-a-pointer is adding unnecessary complexity to the program and programming it. You'd have to be very careful about dereferencing it. I believe you could accomplish the same thing much more easily if LineCopy is a regular-old-char-pointer. The same is true of abs_path_COPY.
2. Use of sizeof in mallocs, part I. This char** LineCopy = malloc((sizeof(char) * strlen(line)) + 1); is redundant. since sizeof(char) is 1, the computer would do less work, you would do less typing, reader would have less thinking with malloc(strlen(line) + 1).

3. Use of sizeof in mallocs, part II. The rest of them are problematic in a different way. For example: char** abs_path_COPY = malloc(sizeof(char**));. char** is a pointer. sizeof(char**) is 4 or 8 depending on architecture. If your request target is "thisisapictureofacat.jpg", well, it's not big enough.

4. strsep and free collide. For example, after this executes:

        char* method = malloc(sizeof(char*));
        method = strsep(LineCopy, " ");
   

   method will be a pointing to an address somewhere within LineCopy. Then when this executes:

        free(method); 
        free(LineCopy); 
   

   it will segfault on the second free, because part of LineCopy has already been freed. Since LineCopy is already allocated, and strsep returns a pointer, this
   char* method = strsep(LineCopy," ");would be the correct usage.
   
   

5. Scope. Pointers. This line abs_path = strsep(LineCopy, "]" + 1); will prevent server from returning a request. Since strsep returns a pointer, this gives the local abs_path a new address. When control returns to main, it will be looking at the original address, and find, well, nothing. You're going to want to look into something like strcpy.

6. This "]" looks like a confusion about the notation used in the documentation here absolute-path [ "?" query ]. There is no actual ']' in the request-line. A fully formed request line with query will look like GET /hello.html?name=Alice HTTP/1.1\r\n" (you could inspect that when you start using gdb). Review this line, too HTTP_version = strsep(LineCopy, "\\");. There is no literal '\' in the request line. '\r' is one character that represents CR.

Wow, that's a lot to digest. Slow and steady. Review the course materials. Reread the specification. Read the man pages. Write small test programs. It will all be worth it.

You might start your journey here:

1. Get rid of all the frees. I think they are causing the seg fault. It will be nigh impossible to make progress if the program is seg faulting.
2. Use gdb. Start small. start gdb as described above, and send a simple curl request in another terminal. something like curl -i http://localhost:8080/hello.html. Nothing with image data to start.

This pset really challenges one to exhibit a solid understanding of pointers and memory allocation and learning to use new functions; coalescing everything learned thus far. It can never be said that it's easy.