What's wrong with my C program using malloc?

Question

This is just the start of a bigger pseudo word processing program that I know needs malloc to run. (Despite a lot of reading, I don't really get malloc).

Anyway, I can get through the loop once. I then enter the second line of text, and it will tells me how long that 2nd line was, but then it crashes. What am I doing wrong?




#include
#include
#include

int len;
int line=0;
char *str_ptr[4]; //create array of character pointers
char buffer[80];

int main()
{

int count=0;

do
{
printf("Enter next line:\n");
fgets(&buffer[0], 79, stdin);
len=strlen (&buffer[0]);
printf("\nThe length of that string was %i\n", len);
str_ptr[count]=(char *) malloc( (len+1) * (sizeof(char)));
strncpy(str_ptr[count], buffer,79); //standard function?
printf("You just typed:\n");
puts(str_ptr[count]);
++count;
}
while (count<4);

return 0;

}

Answer 1

Well, you need to check the return value of fgets. If it returns 0 (NULL), then you should break out of the do-while block.



strncpy is a standard function, but you're using it incorrectly. It pads the rest of dest with '\0' when it reaches the end of the src string, until it's written the number of bytes given to it as a size. Since you give it a size greater than the number of bytes you allocate to str_ptr[count], it will overflow dest. I suggest you read up on how to use it: http://pubs.opengroup.org/onlinepubs/7908799/xsh/strncpy.html



Anyway, since you allocate enough space for the string, then you may as well just strcpy or memcpy buffer to str_ptr[count].



The next problem you have is that you don't free the memory you allocate through malloc. You can fix this by just calling free after you print the string to stdout, or at a later time.



Another problem, is that you don't check check whether malloc passed a null pointer back to you or not. If it did, then you strcpy it, you'll have the same problem that you have with using strncpy. (You'll be trying to access memory that you're not allowed to.)



Here's a copy of the changes I've made:



#include

#include

#include



int main(void)

{

        char *strptr[4];

        char buffer[80];

        size_t count;

        size_t i;



        for (count = 0; count < sizeof strptr / sizeof strptr[0]; count++) {

                size_t size;

               

                printf("Enter a line: ");

                fflush(stdout);

                if (!fgets(buffer, sizeof buffer, stdin))

                        break;

                size = strlen(buffer) + 1;

                strptr[count] = malloc(size * sizeof *strptr[count]);

                if (!strptr[count]) {

                        fprintf(stderr, "Unable to allocate memory\n");

                        return EXIT_FAILURE;

                }

                strcpy(strptr[count], buffer);

                printf("You just typed %s. Its length is %lu.\n",

                strptr[count], (unsigned long) size);

        }

        for (i = 0; i < count; i++)

                free(strptr[i]);

        return 0;

}

Answer 2

You have 2 mistakes:

1. You allocate a memory of size 'len + 1' characters, while use 79 characters of it (with 'strncpy' function). You must allocate more, or use less,

2. You use 'strncpy' function to copy a string to another, while 'strncpy' function will not append null character to end of string. Instead use 'strcpy' function to copy strings.



good luck

Answer 3

I believe the 1st answerer. Why use char **buffer? attempt it with one asterik. additionally, upload a million greater byte to the malloc to go away room for the null character. char *buffer = NULL; fgets(line, BUFSIZE, fp); lengthOfLine = strlen(line); buffer = (char*)malloc(lengthOfLine + a million); strcpy(buffer, line);