malloc is not working properly?

Question

#include<stdio.h>
#include<stdlib.h>
void func(char *p[],int);
void main()
{
  char **strings;
  int r;
  printf("\nEnter the no. of rows :");
  scanf("%d",&r);
  strings=malloc(r*sizeof(strings));
  func(strings,r);
  printf("%s %s\n", strings[0], strings[1]);
}

void func(char *p[],int r)
{
   int i;
   for(i=0;i<r;i++)
      scanf("%s",p[i]);
}

This code does not take more than 3 inputs ...after 3 inputs , its stopping abruptly.

Why is malloc not working properly? Is it only because of malloc ?

Can you please edit your question to add the exact input you entered, the output you got and the output you expected? It's necessary to understand how the code is supposed to work. — Bob__, Oct 13 '16 at 16:12
Rule of thumb with a language as mature and as easy to get wrong as C, any variation of "the compiler is broken" is a last resort. More than likely it's your code. — Schwern, Oct 13 '16 at 18:36

score 4 · Answer 1 · answered Oct 13 '16 at 18:47

The problem is in how you're using malloc. You're not allocating enough memory, and you can't know how much memory to allocate.

strings=malloc(r*sizeof(strings));

This allocates enough memory in strings for r character pointers. strings can now safely store r variables of type char *. Trouble is you never allocate memory for the strings stored in strings.

void func(char *p[],int r)
{
   int i;
   for(i=0;i<r;i++)
      /* At this point, p[i] is unallocated */
      scanf("%s",p[i]);
}

p[i], the thing actually storing the string, also has to be allocated. How much? Who knows, you're reading from input using scanf. So first cut is to allocate a fixed size and only read at most that size (less one because of the null character).

void func(char *p[],int r)
{
   int i;
   for( i=0; i < r; i++ ) {
       p[i] = malloc( 256 * sizeof(char) );
       scanf("%255s",p[i]);
   }
}

Never use scanf %s without a max length else the input can be larger than the memory you allocated. Also don't use scanf, it can get you into trouble if the input doesn't match your pattern. It's better to read the whole line with getline or fgets and then sscanf if necessary.

A memory checker is absolutely essential when coding in C. It will find these problems for you. I use valgrind.

Enter the no. of rows :5
foo
==10082== Use of uninitialised value of size 8
==10082==    at 0x1001F2121: __svfscanf_l (in /usr/lib/system/libsystem_c.dylib)
==10082==    by 0x1001EA979: scanf (in /usr/lib/system/libsystem_c.dylib)
==10082==    by 0x100000F2B: func (test.c:19)
==10082==    by 0x100000EBD: main (test.c:11)
==10082== 
==10082== Invalid write of size 1
==10082==    at 0x1001F2121: __svfscanf_l (in /usr/lib/system/libsystem_c.dylib)
==10082==    by 0x1001EA979: scanf (in /usr/lib/system/libsystem_c.dylib)
==10082==    by 0x100000F2B: func (test.c:19)
==10082==    by 0x100000EBD: main (test.c:11)
==10082==  Address 0x0 is not stack'd, malloc'd or (recently) free'd

Those are stack traces pointing at memory problems. "Use of uninitialised value of size 8" tells me I failed to allocate memory for a string. The stack tells me it's the scanf in func.

Why not create a temporary `char` array to store each scanned `char*` into then `malloc` the length of each pointer (+1 for null)? A fixed size and caution in not over stepping the bounds would still be needed but it seems like it will save some memory especially in a much larger scale of this. — duncan, Oct 13 '16 at 18:56
@duncan Yes, you can have one `char line[256]` to read the lines and copy just the, probably very small, content into `p`. `getline()` goes one better and handles allocating `line` for you. — Schwern, Oct 13 '16 at 20:28
@schwern , why did u specifically use 256 or 255 over there ? — shashank, Oct 14 '16 at 14:14
@shashank 256 is longer than any valid input is likely to be, but not so long as to be a waste of memory. It's not a great way to read lines, I just wanted to show where you needed to allocate memory. The better solution is to use `getline` which will allocate memory for the line for you. — Schwern, Oct 14 '16 at 17:32

malloc is not working properly?

1 Answers1