0

Just after completing Parse i've stucked on load function. By cs50 check i definately understand that something is wrong in reading memory to a buffer and maybe even accsessing it. For my eyes this code looks ok, but it definately wrong so i am asking for help to find those bad places and explanation to what is wrong and with what tools could it be fixed. Hope it will be working soon with your help!

Here is link to check50: https://sandbox.cs50.net/checks/b81df3c25c9c403b906e1ec0ce2fce1a

So, here is my sweet load function:

bool load(FILE* file, BYTE** content, size_t* length)
{
    if (file == NULL)
    {
    return false;
    }
    *content = NULL;
    *length = 0;


    BYTE* buffer = malloc(sizeof(BYTE));
    if (buffer == NULL)
    {
        free(buffer);
        return false;
    }
    int len = 0;
    while(fread(buffer, 512, 1, file) == 1)
    {
        *content = realloc(buffer, (len + 1) + sizeof(BYTE));
        len++;
    }

    *length = len;

    return true;
}

I will also paste index function, as maybe checker is yelling because of it:

char* indexes(const char* path)
{
    char* html = "/index.html";
    char* php = "/index.php";
    char *alt_html_path = malloc(strlen(path) +strlen(html) +1);
    char *alt_php_path = malloc(strlen(path) +strlen(php) +1);
    strcpy (alt_html_path, path);
    strcat (alt_html_path, html);
    strcpy (alt_php_path, path);
    strcat (alt_php_path, html);

    //if (index.html) return /path/to/a/directory/index.html
    if ( access(alt_html_path, F_OK) != -1 ) 
    {
        free(alt_php_path);
        return alt_html_path;
    }
    //if (index.php) return /path/to/a/directory/index.php
    else if ( access(alt_php_path, F_OK) != -1)
    {
        free(alt_html_path);
        return alt_php_path;
    }
    else
    {
        free(alt_php_path);
        free(alt_html_path);
        return NULL;
    } 
}

1 Answer 1

1

load

buffer is allocated for one byte here BYTE* buffer = malloc(sizeof(BYTE));

This while(fread(buffer, 512, 1, file) == 1) reads 512 bytes into buffer.

And buffer is only and ever allocated for 1 byte; this *content = realloc(buffer, (len + 1) + sizeof(BYTE)); doesn't reallocate buffer. It reallocates the memory pointed to by buffer and stores the "new" pointer in *content.

len will be incremented for each 512-byte chunk of file. So cat.jpg at 26860 bytes will return 52 for *length.

You might want to contemplate how fread(buffer, 1, 512, file) could help. When the size argument is 1, fread returns the number of bytes read.

indexes

Review the specification for indexes:

indexes

Complete the implementation of indexes in such a way that the function, given a /path/to/a/directory, returns /path/to/a/directory/index.php if index.php actually exists therein, or /path/to/a/directory/index.html if index.html actually exists therein, or NULL. In the first of those cases, this function should dynamically allocate memory on the heap for the returned string.

Order matters.

And this looks like a typo strcat (alt_php_path, html);.


modified load (from comment)

int len = 0;
 BYTE* buffer = malloc(sizeof(BYTE));
 while(fread(buffer, 1, 512, file) == 1)
 { buffer = realloc(buffer, sizeof(BYTES)*(len + 1));
 len++; }
 *content = buffer;
 *length = len;

Problems:

  1. buffer is initially allocated for 1 byte, ie sizeof(BYTE), and then fread tries to stuff 512 bytes into it. Perhaps you meant sizeof(BYTES), but that is wrong - read on to #2. In any case, if you plan to read a maximum of 512 bytes into buffer, then it should (must?) be allocated for at least 512 bytes.
  2. sizeof(BYTES) may not be what you think it is. I think you think it is 512. It is true that BYTES is 512, which is an int. sizeof(an int) is 4 or 8 depending on architecture.
  3. len is being incremented once for every 512 bytes read. So as before, cat.jpg at 28680 bytes will compute len as 52. *length must match the number of bytes in the file exactly. You can see what to expect with ls -al command. For instance, ls -al cat.jpg will return
    -rw-rw-r-- 1 ubuntu ubuntu 26860 Oct 24 2015 cat.jpg where 26860 is the file size.
  4. fread is always storing what it reads into the first byte of buffer. So the result of *content will be the last chunk of bytes that were read. You need to "push it along", ie read into the newly allocated space in buffer. If you know how many total bytes have been read, then you know where the "end" of buffer is.
  5. Remember that this fread(buffer, 1, 512, file) command will return the number of bytes read (or 0 if EOF encountered). Consider a small change to your while condition, such as
    while((len = fread(buffer, 1, 512, file)) != 0). You'll still need arithmetic to accumulate it into *length and to "push" the buffer pointer to the end.

You have a good start here, you are on your way to a perfect finish. You can use gdb to help with the details. In a terminal, start gdb:

$ gdb ./server
(gdb) break load
(gdb) run ./public

In a second terminal send a curl request for hello.html curl -i http://localhost:8080/hello.html. Back in the gdb terminal it should be waiting at the load function. Let the troubleshooting begin! IMO curl is easier than a browser call because it doesn't bother with favicon and hello.html is a good starting file because it is small (316 bytes) and it is text, i.e. human-readable. Beware, however, the size is a double edged sword, since it will only do one iteration of the while loop.

17
  • Well, i've tried to manage with load but with no luck. Here how i understand the next code is working: int len = 0; BYTE* buffer = malloc(sizeof(BYTE)); while(fread(buffer, 1, 512, file) == 1) { buffer = realloc(buffer, sizeof(BYTES)*(len + 1)); len++; } *content = buffer; *length = len; Dec 2, 2016 at 18:37
  • 1) declare counter for length, then we maaloc 1 byte, 2) fread 512 blocks of 1 byte size and if we had no space we realloc our existing bufer to grow in size of 512(as defined in BYTES)*(len+1). 3) dereference content to point to our buffer, length to our len. Dec 2, 2016 at 18:46
  • could you explain what's wrong in my thougts? Dec 2, 2016 at 18:47
  • Answer amended. Dec 2, 2016 at 21:49
  • Well, things go much more better. Thank you @DinoCoderSaurus. The fail is i dont quite understand the following line: len = fread(buffer - end_of_buff, 1, 512, file)) !=0 if i put - it gives me next sandbox - sandbox.cs50.net/checks/621aaf333e8c437591e0a66991037ab8 and for + - sandbox.cs50.net/checks/411ebc293d5c41b192eb8f4aa8969cb5 Dec 3, 2016 at 0:29

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .