dynamically allocate string using malloc

Question

I am new to C programming. Right now I am learning string and pointers.
As a beginner I find it difficult to find the mistake. I have written a code for dynamic allocation of a string and print the string using function.the code is given below.

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

 void fun (char *);

 int main()
 {
    int n;
    char *s = NULL;
    printf("enter the length of the string\n");
    scanf("%d",&n);
    s  = (char*)malloc(n * sizeof(char));
    printf("enter the string\n");
    fgets(s, 20, stdin);
    printf("string before  passing is %s\n",s);
    fun(s);
    return 0;
 }

 void fun( char *p)
 {
    char *d;
    d = p;
    printf("string after passing is%s\n",d);      
 }

There is no error message while compiling. But the code doesn't take string.
Can anyone please help me in finding the mistake.

Answer 1

You have five major bugs in your code.

1. scanf will not "consume" the newline generated when you hit ENTER, so you'll need getchar to follow it to handle this unexpected behavior.
2. You're not checking the result of malloc. It is possible for this function to fail.
3. fgets should use n to determine the maximum amount of data to take from the user, or you'll generate a segmentation fault by overflowing your buffer.
4. The string containing the result of fgets ends with a newline character AND a NULL terminating character in typical cases. You can use strtok to replace the trailing newline with a NULL character to avoid unexpected behaviors.
5. You are not freeing the memory you created with malloc. This leads to memory leaks.

EDIT: Matt McNabb provides an alternative approach to handling the unconsumed \n in FIX1, which I think is a cleaner approach than mine.

The solution is posted below.

Code Listing

---

#include<stdio.h>
#include<stdlib.h>
#include<string.h>
void fun (char *);

int main() {
   int n;
   char *s = NULL;
   printf("enter the length of the string\n");
   scanf("%d",&n);
   (void)getchar();     // FIX1

   s  = (char*)malloc(n * sizeof(char));
   if (s == NULL) {     // FIX2
      printf("malloc error; aborting\n");
      return 1;
   }
   printf("enter the string\n");
   fgets(s, n, stdin);  // FIX3
   strtok(s, "\n");     // FIX4
   printf("string before  passing is %s\n",s);
   fun(s);
   free(s);             // FIX5
   return 0;
}

void fun(char *p) {
   char *d;
   d = p;

   printf("string after passing is %s\n",d);
}

Sample Run

---

enter the length of the string
10
enter the string
This is a test for overflows.
string before  passing is This is a
string after passing is This is a

Answer 2

scanf("%d",&n);

After you entered the number and press ENTER, the number assumed by this scanf line but the new line character is still in buffer. The next fgets line takes only that new line.

One simple way to fix the problem is:

scanf("%d",&n);
getchar();      //consume the new line

Answer 3

When you write scanf("%d",&n); , it extracts an integer from the stream. It does not discard the rest of the line. So if you typed a number and pressed enter, then the newline remains in the stream .

So when you do fgets it reads the rest of that line, which is blank.

To fix this you need to read up to the end of the line. One way to do that is:

int ch;
while ( (ch == getchar()) != '\n' && ch != EOF ) { }

Then you can go on to do:

fgets(s, n, stdin);   // n, not 20

Also, you should check the result of malloc, e.g.

 s  = malloc(n);
 if ( s == NULL )
     return 0;   // or some other error handling

Casting malloc has been a mistake since 1989, but it's a matter of style whether you want to include the sizeof(char) or not, since sizeof(char) is always 1.

To be complete you could also check the result of fgets, for example if your input was closed then you would be outputting garbage.

Answer 4

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

void fun (char *);

int main()
{
        int n;
        char *s = NULL;
        char ch;

        /* Taking String length from the User */
        printf("enter the length of the string\n");
        scanf("%d",&n);

        /* Allocating Memory */
        s = (char*)malloc(n * sizeof(char));

        /* Checking for memory allocation */
        if (s == NULL)
        {
                printf("\n Error in allocating memory \n");
                exit(0);
        }

        /* Flush standard inputs */
        while ((ch = fgetc(stdin)) != EOF && ch != '\n')
        {
        }

        printf("enter the string\n");
        fgets(s, n+1, stdin);

        printf("string before  passing is %s\n",s);
        fun(s);

        /* Need to free the allocated memory */
        free(s);
        return 0;
}

void fun( char *p)
{
        char *d;
        d = p;
        printf("string after passing is %s \n",d);
}

Answer 5

in short, fgets reads from buffer and buffer is already full. Solution is not to use fgets or flush buffer before reading

printf("enter the string\n");
fflush(stdin);
fgets(s, 20, stdin);

UPD

so if fflush is undefined for stdin, then flush it as Matt McNabb mentioned above

while ((ch = getchar()) != '\n' && ch != EOF);