[SOLVED] Beginning C programming- learning basic use of malloc/realloc

keithostertag · 02-28-2012, 12:03 AM

I've been using variations of this code to learn the proper way to use malloc, realloc, and pointers. This program takes keyboard input and prints out the index number and character entered. My intention is to add memory as needed each time the small buffer is full.

The program crashes at the 28th iteration. I have tried to use gdb to see the problem, but no joy. I also have used valgrind a bit- valgrind reports "invalid write size of 1" each time I call realloc. Valgrind also says "Address xxxxxxxxxx is 0 bytes after a block of size 4 alloc'd".

Is this an index problem, still? I have tried to carefully follow each statement but I need help seeing the problem.

Of course any other comments to help improve the code/style is welcome.

Thanks,
Keith Ostertag

Code:

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

int sizeOfBuffer;
const int initialSize = 4;
int memoryAllocationProblem(void);
int additionalMemory = 4;

int main(void)
{
char *buffer, *tmp;
int x=0;

sizeOfBuffer = initialSize * sizeof(char);
if ((buffer = (char *) malloc(sizeOfBuffer)) == NULL)
	memoryAllocationProblem();   /* in case of a problem */

while (x != sizeOfBuffer && (buffer[x]= getchar()) != '\n')
	{	
	printf("\tbuffer[%d]= %c\n", x, buffer[x]);
	x++;
	if (x==sizeOfBuffer)
		{	
		tmp = realloc(buffer, additionalMemory);
    		if (tmp != NULL) buffer = tmp; else memoryAllocationProblem();
		sizeOfBuffer += additionalMemory; 
    		}
	}
free(buffer);
return 0;
}

int memoryAllocationProblem(void)
{
	puts("Memory allocation problem!\n");
	abort();
}

Here is sample output:

Code:

keith@t520:~/cprograms$ tst2-7 
abcdefghijklmnopqrstuvwxyzabcdefg
	buffer[0]= a
	buffer[1]= b
	buffer[2]= c
	buffer[3]= d
	buffer[4]= e
	buffer[5]= f
	buffer[6]= g
	buffer[7]= h
	buffer[8]= i
	buffer[9]= j
	buffer[10]= k
	buffer[11]= l
	buffer[12]= m
	buffer[13]= n
	buffer[14]= o
	buffer[15]= p
	buffer[16]= q
	buffer[17]= r
	buffer[18]= s
	buffer[19]= t
	buffer[20]= u
	buffer[21]= v
	buffer[22]= w
	buffer[23]= x
	buffer[24]= y
	buffer[25]= z
	buffer[26]= a
	buffer[27]= b
*** glibc detected *** tst2-7: realloc(): invalid next size: 0x0000000001b55010 ***
======= Backtrace: =========
/lib/x86_64-linux-gnu/libc.so.6(+0x75ab6)[0x7f70758ceab6]
/lib/x86_64-linux-gnu/libc.so.6(+0x7b68c)[0x7f70758d468c]
/lib/x86_64-linux-gnu/libc.so.6(realloc+0xf0)[0x7f70758d49a0]
tst2-7[0x400740]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xfd)[0x7f7075877ead]
tst2-7[0x4005f9]
======= Memory map: ========
00400000-00401000 r-xp 00000000 08:01 4325917                            /home/keith/cprograms/tst2-7
00600000-00601000 rw-p 00000000 08:01 4325917                            /home/keith/cprograms/tst2-7
01b55000-01b76000 rw-p 00000000 00:00 0                                  [heap]
7f7070000000-7f7070021000 rw-p 00000000 00:00 0 
7f7070021000-7f7074000000 ---p 00000000 00:00 0 
7f7075643000-7f7075658000 r-xp 00000000 08:01 2883589                    /lib/x86_64-linux-gnu/libgcc_s.so.1
7f7075658000-7f7075858000 ---p 00015000 08:01 2883589                    /lib/x86_64-linux-gnu/libgcc_s.so.1
7f7075858000-7f7075859000 rw-p 00015000 08:01 2883589                    /lib/x86_64-linux-gnu/libgcc_s.so.1
7f7075859000-7f70759d6000 r-xp 00000000 08:01 2883588                    /lib/x86_64-linux-gnu/libc-2.13.so
7f70759d6000-7f7075bd6000 ---p 0017d000 08:01 2883588                    /lib/x86_64-linux-gnu/libc-2.13.so
7f7075bd6000-7f7075bda000 r--p 0017d000 08:01 2883588                    /lib/x86_64-linux-gnu/libc-2.13.so
7f7075bda000-7f7075bdb000 rw-p 00181000 08:01 2883588                    /lib/x86_64-linux-gnu/libc-2.13.so
7f7075bdb000-7f7075be0000 rw-p 00000000 00:00 0 
7f7075be0000-7f7075bff000 r-xp 00000000 08:01 2883639                    /lib/x86_64-linux-gnu/ld-2.13.so
7f7075de5000-7f7075de8000 rw-p 00000000 00:00 0 
7f7075dfb000-7f7075dff000 rw-p 00000000 00:00 0 
7f7075dff000-7f7075e00000 r--p 0001f000 08:01 2883639                    /lib/x86_64-linux-gnu/ld-2.13.so
7f7075e00000-7f7075e01000 rw-p 00020000 08:01 2883639                    /lib/x86_64-linux-gnu/ld-2.13.so
7f7075e01000-7f7075e02000 rw-p 00000000 00:00 0 
7fff99122000-7fff99143000 rw-p 00000000 00:00 0                          [stack]
7fff991ff000-7fff99200000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]
Aborted

everest40 · 02-28-2012, 12:46 AM

I don't know for sure if this is causing the problem you're encountering, but it looks to me like you're misunderstanding what the realloc() function does. You seem to think that its second argument (the size of the memory to be allocated) will add to what has already been allocated, thus always expanding the amount of memory available for use.

Quoting from http://www.cplusplus.com/reference/c...tdlib/realloc/ (a site which provides very good references for both C and C++) (emphasis mine)

Quote:

void * realloc ( void * ptr, size_t size );

Reallocate memory block
The size of the memory block pointed to by the ptr parameter is changed to the size bytes, expanding or reducing the amount of memory available in the block... The content of the memory block is preserved up to the lesser of the new and old sizes, even if the block is moved.

So, your realloc() function call should be changed from

Quote:

tmp = realloc(buffer, additionalMemory);

to something like

Quote:

tmp = realloc(buffer, sizeOfBuffer + additionalMemory);

Dark_Helmet · 02-28-2012, 01:28 AM

everest40 is correct. And I believe that's the source of your problem. I'll post some modified code in a moment that addresses his point, but first...

Quote:

Originally Posted by keithostertag

Of course any other comments to help improve the code/style is welcome

I mean this in the nicest possible way: the formatting you use makes me cry.

I don't know why (for everything that is sacred in this world) that textbooks or tutorials use examples that place functional code inside loops and/or conditions. From my perspective, it's dumb and serves only to confuse people learning the subject--as evidenced by one of your last questions where the ordering of the for loop condition caused your problem.

Break the code up into teeny, tiny pieces. You will love yourself for it later if you get in the habit now.

Now, you may not like my indentation style or curly brace style, but take an objective look at the code I post and the code you posted. Ask yourself, "which is easier to understand/read?"

The changes (as indicated by everest40) are in red. Everything else should be functionally equivalent to your original. (As a side note, I think you would learn more if you wrote the memory reservation routine from scratch rather than continuing to build on your buddy's code).

Code:

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

int sizeOfBuffer;
const int initialSize = 4;
int memoryAllocationProblem(void);
int additionalMemory = 4;

int main(void)
{
  char *buffer;
  char *tmp;
  int   x=0;

  sizeOfBuffer = initialSize * sizeof(char);

  buffer = (char *) malloc( sizeOfBuffer );

  if( buffer == NULL )
    memoryAllocationProblem();   /* in case of a problem */

  do
  {
    buffer[x] = getchar();

    if( buffer[x] == '\n' )
      break;

    printf( "\tbuffer[%d] = %c\n", x, buffer[x] );

    x++;

    if( x == sizeOfBuffer )
    {
      sizeOfBuffer += additionalMemory;  /* placement changed */

      tmp = realloc( buffer, sizeOfBuffer );

      if( tmp == NULL )
	memoryAllocationProblem();

      buffer = tmp;
    }

  } while( 1 );

  free(buffer);

  return 0;
}

int memoryAllocationProblem(void)
{
  puts( "Memory allocation problem!\n" );
  abort();
}

sundialsvcs · 02-28-2012, 09:21 AM

Also, I encourage you to think of these low-level explorations merely as an exercise, and to plan forthwith to use C++ for most of your actual work. They are profitable enough as an exercise, but the overall techniques are simply too error-prone.

keithostertag · 02-28-2012, 10:30 AM

Thank you both, everest40 and Dark_Helmet for your generous and gentle replies.

everest40- Yes, you are exactly right, I was mis-reading the definition of realloc.

I generally read the manpage, which says: " The realloc() function changes the size of the memory block pointed to by ptr to size bytes." I was mis-reading it to mean "pointed to by ptr by size bytes".

And I will make use of the link to cplusplus as well.

Dark- thanks once again for your comments about style. If only you (and others) will continue to show me, I will eventually learn...if I could afford to pay you as a private tutor I certainly would

Edit:
sundialsvcs- Well, one thing at a time. I want to see if I can do well with these exercises before I even consider another language. I'm not particularly attracted to C++, and I am only doing this for fun and stimulation.

Keith Ostertag

Dark_Helmet · 02-28-2012, 11:08 AM

I was a little concerned you might have taken the "cry" comment the wrong way. It appears not, which certainly does not make me cry

Also, I appreciate the compliment, but if you were going to pay someone, there are far more deserving people on this forum than some weirdo in a helmet.

And just for clarification on something I said earlier:

Quote:

I don't know why . . . textbooks or tutorials use examples that place functional code inside loops and/or conditions.

Certainly, functional code inside a loop is not a bad thing. What I was referring to was the loop construct--not the interior of the loop itself (e.g. "while( x = getch() != '\n' )" or "for( x = 0; buffer[x] = getch(); buffer++ )").

I have a tendency toward a "do one small, simple thing at a time" philosophy.