Thread: Dynamic memory allocation problem

WHY ARE YOU YELLING!?!? AND WHY HAVEN'T YOU READ ALL THE RULES YET???

C Board - Announcements in Forum : C Programming
<< !! Posting Code? Read this First !! >>

But it is important. I don't want to start a giant argument linuxlover, but you're the one coming to us for help. Generally, you want to be nice to the people you help, and make it as easy as possible for them to help you, especially since many of us spend our "break time" at work doing this. Keep that in mind when you post, and make your posts as easy to read as possible. That would include proper English grammar, spelling and capitalization to the best of your ability, and code tags with properly formatted code. Yes, we realize many people are not native English speakers (there are many non-English boards out there too, if your English is that inhibitory -- yours isn't), and we try to be as sympathetic to that as possible, but like Salem mentioned, you have 14 posts, most of the correct, so you must have some idea of how this generally works.

And lighten up. Nobody here is really mean, it's just that there isn't a [sarcasm] tag.

Originally Posted by anduril462

But it is important. I don't want to start a giant argument linuxlover, but you're the one coming to us for help. Generally, you want to be nice to the people you help, and make it as easy as possible for them to help you, especially since many of us spend our "break time" at work doing this. Keep that in mind when you post, and make your posts as easy to read as possible. That would include proper English grammar, spelling and capitalization to the best of your ability, and code tags with properly formatted code. Yes, we realize many people are not native English speakers (there are many non-English boards out there too, if your English is that inhibitory -- yours isn't), and we try to be as sympathetic to that as possible, but like Salem mentioned, you have 14 posts, most of the correct, so you must have some idea of how this generally works.

And lighten up. Nobody here is really mean, it's just that there isn't a [sarcasm] tag.

OK I understood..

Okay, you're still yelling in your original post (using all capital letters), and I'm not 100% sure what you're after, but here are some problems I noticed and some (strong) suggestions:
1. Declare main to explicitly return an int.
2. Don't cast the result of a malloc/calloc/realloc.
3. Don't increment p as you go along, you lose the starting address of the memory you allocated and free(p) will take a big ugly dump all over everything. Try using buf as the result of malloc/realloc and using an integer to index into buf (you can't just increment p since realloc doesn't guarantee the memory starts at the same location).
4. Don't use the same pointer for realloc, since it may return NULL, then you lose your originally allocated memory too.
5. Calling realloc, increasing 1 byte at a time is very inefficient. Either keep track of the size of what you've allocated, and when you reach the end, allocate a big chunk more, like:

Code:

if (num_of_chars_read == size_allocated) {
    temp = realloc(p, size_allocated + 1000;
    if (!temp) {
        printf("ERROR: ran out of memory on the heap!\n");
        return 1;
    }
    p = temp;
    size_allocated += 1000;
}

6. You keep setting p to the start of the reallocated memory, which means as you write into p, you're only writing the first character each time.
7. You're forcing your buffer to start with 'y'. I don't think that's necessary or a good idea. Try a do-while loop.
8. You never increment count when you printf(..., p+count), you're always printing *(p+0) or just *p.

That's all I have time for right now. Again, I'm not totally clear on your problem description, and if you aren't either, we're both going to have serious problems with this program.

Originally Posted by anduril462

Okay, you're still yelling in your original post (using all capital letters), and I'm not 100% sure what you're after, but here are some problems I noticed and some (strong) suggestions:
1. Declare main to explicitly return an int.
2. Don't cast the result of a malloc/calloc/realloc.
3. Don't increment p as you go along, you lose the starting address of the memory you allocated and free(p) will take a big ugly dump all over everything. Try using buf as the result of malloc/realloc and using an integer to index into buf (you can't just increment p since realloc doesn't guarantee the memory starts at the same location).
4. Don't use the same pointer for realloc, since it may return NULL, then you lose your originally allocated memory too.
5. Calling realloc, increasing 1 byte at a time is very inefficient. Either keep track of the size of what you've allocated, and when you reach the end, allocate a big chunk more, like:

Code:

if (num_of_chars_read == size_allocated) {
    temp = realloc(p, size_allocated + 1000;
    if (!temp) {
        printf("ERROR: ran out of memory on the heap!\n");
        return 1;
    }
    p = temp;
    size_allocated += 1000;
}

6. You keep setting p to the start of the reallocated memory, which means as you write into p, you're only writing the first character each time.
7. You're forcing your buffer to start with 'y'. I don't think that's necessary or a good idea. Try a do-while loop.
8. You never increment count when you printf(..., p+count), you're always printing *(p+0) or just *p.

That's all I have time for right now. Again, I'm not totally clear on your problem description, and if you aren't either, we're both going to have serious problems with this program.

My concept is this about realloc..

realloc(p,no. of bytes) resizes the previously allocated memory whose address of first element is p, to a newsize specified by "no. of bytes".This may use the previous continues memory region or if it is not possible to use that region,it will copy the contents of previously allocated region to a new location whose size is "no of bytes".

ie. the pointer may or may not contain previous address ,but it will point to the same data....

p = (char*)malloc(sizeof(char)) allocates one byte of memory and returns a pointer to starting address.
suppose *p holds 's'.
if i use realloc (p,2) the size of dynamic memory whose address of first element is stored in p is resized to 2 bytes.The initial address will again point to memory containing 's'
ie. The size is changing but the memory pointer will point to the same data(memory location may or may not be same)

free(p) is used to free the allocated memory .But it is used after the printf() .So what is the problem.


My for loop has changed like this for(i=0;*(p+i)!='\n';++i)
So it will not point to starting address na..

*p='y' is used for giving initial value


count is incremented

Corrections are made in new attachment .
But again problem persists.


What is the problem of type casting here?

I still have no idea what your code is really trying to do, and, no offense, but your logic is very screwy and your code very difficult to understand. Thus, my suggestions are very general, and I'm not sure I can address your problem since I don't know what your code should be doing and what it's doing wrong. Lots of clear, explicit details would be helpful. What is the full problem description? What do you mean by

and print it until heap is empty.

On another side note, it is generally considered bad form to go back and "revise history" in a forum. A quick edit for a typo or reformatting is one thing, but replacing attachments and deleting your original code posting is not a good idea. I don't have your original code to compare your changes with, which would be handy (I might still have a copy on my work computer, but I wont be able to get to it until 8AM California time -- 9 hours from now). Now, down to business...


Your concept of realloc is fine, if a tiny bit incomplete (see item 4 below). Let's go over your handling of the 8 suggestions I made:
1. Resolved
2. There is nothing wrong with it, but it's considered bad style, since it's outdated, unnecessary and clutters stuff up.
3. Not an issue in your new code, but I no longer have your old code to refer to or compare with.
4. This is still an issue. The following quote is from the man page for realloc:

realloc() returns a pointer to the newly allocated memory, which is suitably aligned for any kind of variable and may be different from ptr, or NULL if the request fails....If realloc() fails the original block is left untouched; it is not freed or moved.

So, if any one of your realloc calls fails, p will have a value of NULL, since you set it to the result of realloc every time. You now lost the address of the block of memory you had before realloc, which was valid, but now you can't print anything out or free it.
5. Still outstanding
6. No longer a problem
7. You don't have to fix this, but it's totally unnecessary. As a matter of fact, you could totally skip the initial malloc and *p = y if you switched to a do-while loop and set your initial values correctly. It would also greatly simplify your code.
8. Resolved.

After running this through a debugger, you have some problems with the way you handle reading your first and second characters. The reason you aren't seeing anything is because your program is stuck in a scanf statement, waiting for more input, and when it does read another character, it hits the end of the for loop, increments i and then you check the wrong character (1 position too far) for '\n'. Simply changing the for loop to check *(p+1-1) != '\n' isn't a good idea however, since i will be 0 the first time this check is performed, and you will effectively be asking for p[-1], and negative array indices are illegal.

You also don't check to see whether realloc succeeds or fails until you're completely done taking input. You need to check this result on every call to realloc, and use a temp variable in case realloc fails. Otherwise, your code may end up trying to store a scanned character into a null pointer (since p == 0, and p+i is very likely an invalid address). That's a seg fault just waiting to happen.

Originally Posted by anduril462

I still have no idea what your code is really trying to do, and, no offense, but your logic is very screwy and your code very difficult to understand. Thus, my suggestions are very general, and I'm not sure I can address your problem since I don't know what your code should be doing and what it's doing wrong. Lots of clear, explicit details would be helpful. What is the full problem description? What do you mean by

On another side note, it is generally considered bad form to go back and "revise history" in a forum. A quick edit for a typo or reformatting is one thing, but replacing attachments and deleting your original code posting is not a good idea. I don't have your original code to compare your changes with, which would be handy (I might still have a copy on my work computer, but I wont be able to get to it until 8AM California time -- 9 hours from now). Now, down to business...


Your concept of realloc is fine, if a tiny bit incomplete (see item 4 below). Let's go over your handling of the 8 suggestions I made:
1. Resolved
2. There is nothing wrong with it, but it's considered bad style, since it's outdated, unnecessary and clutters stuff up.
3. Not an issue in your new code, but I no longer have your old code to refer to or compare with.
4. This is still an issue. The following quote is from the man page for realloc:

So, if any one of your realloc calls fails, p will have a value of NULL, since you set it to the result of realloc every time. You now lost the address of the block of memory you had before realloc, which was valid, but now you can't print anything out or free it.
5. Still outstanding
6. No longer a problem
7. You don't have to fix this, but it's totally unnecessary. As a matter of fact, you could totally skip the initial malloc and *p = y if you switched to a do-while loop and set your initial values correctly. It would also greatly simplify your code.
8. Resolved.

After running this through a debugger, you have some problems with the way you handle reading your first and second characters. The reason you aren't seeing anything is because your program is stuck in a scanf statement, waiting for more input, and when it does read another character, it hits the end of the for loop, increments i and then you check the wrong character (1 position too far) for '\n'. Simply changing the for loop to check *(p+1-1) != '\n' isn't a good idea however, since i will be 0 the first time this check is performed, and you will effectively be asking for p[-1], and negative array indices are illegal.

You also don't check to see whether realloc succeeds or fails until you're completely done taking input. You need to check this result on every call to realloc, and use a temp variable in case realloc fails. Otherwise, your code may end up trying to store a scanned character into a null pointer (since p == 0, and p+i is very likely an invalid address). That's a seg fault just waiting to happen.

Thank you for finding the mistake in for loop ....
I resolved my problem
There were some other mistakes also .But finally I resolved all of them and gave output..Please tell me if there is any exceptional input..

Is heap memory that much large (it never gave a message "NO SPACE IN HEAP MEMORY" instead printed the input itself)
Now please resolve my doubts about heap memory given with the problem

Corrected code is attached in a file named test51.c

The new code seems to do what it's supposed to, but it's still a bit messy. Here is what I envisioned (a bit cleaner I think):

Code:

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

int main(void)
{
    int     c,
            i,
            size = 0;
    char    *buf = NULL,
            *temp;

    printf("Enter String: ");

    do {
        c = getchar();
        temp = realloc(buf, size+1);
        if (!temp) {
            printf("No space in heap!\n");
            return 1;
        }

        buf = temp;

        if (c == '\n') {
            buf[size++] = '\0';    // we're done, null terminate the string
        }
        else if (c != EOF) {
            buf[size++] = c;
        }
    } while(c != EOF && c != '\n');

    for (i = 0; i < size; i++) {
        putchar(buf[i]);
    }

    free(buf);

    return 0;
}

Oops, got distracted and forgot to post this:

Memory layout of a process in Linux looks something like this (code tags for formatting):

Code:

+-------+
| Stack |   Function info: params, local variables, return address to calling function, etc
|   |   |
|   v   |
|       |
|       |
|   ^   |
|   |   |
| Heap  |   Dynamically allocated memory
+-------+
| BSS   |   Uninitialized global and static variables
+-------+
| Data  |   Initialized global and static variables
+-------+
| Text  |   Assembly instructions
+-------+

The "arrows" signify that the heap "grows up" and the stack "grows down". Too many nested function calls can definitely cause the stack to grow down into the heap region, causing a seg fault. I don't know for sure whether there is a "hard" limit on heap size, i.e. whether there should be a line in my diagram saying "this is the end of the heap, period" or whether it will keep allocating until it runs into the stack. My guess is there's a hard limit, but due to the large size (megabytes or gigabytes) provided by virtual memory addressing, you are unlikely to hit it, especially when it's driven by user input from the keyboard. I don't know of any way to dynamically increase it, but I don't think you have to worry about that since it's so big.

Regarding who is in charge of the heap, as far as I understand, it's an OS thing. malloc/calloc/realloc are pretty low level calls, and probably talk to the operating system, who handles the allocation and freeing of chunks of memory by the heap. Note that, while there is only the one OS managing the heaps for all processes, there are separate heaps for each process, each with it's own virtual memory space.

Interestingly enough, your use of realloc means you can only ever guarantee space for a string of H/2 - 1 bytes of memory, since realloc must allocate the next chunk before