Sidebar Sidebar
Become a MacRumors Supporter for $50/year with no ads, ability to filter front page stories, and private forums.

macOS C program printing out junk; possible pointer issue

M

mac2x

macrumors 65816
Original poster
Sep 19, 2009
1,146
0
First off, I'm sure this is because I've just started working seriously with pointers. I'm sure I'm doing something amiss.

Extensive testing has told me that the contents of the arrFor are being corrupted somehow between the allocateArray function and the display function. I *suspect* that it has something to do with passing arrFor to the displayResults function. However, I've reached the point where I can't see how to fix it anymore and need outside help. Thanks!

N.B. arrBack remains uncorrupted; that's why i suspect the function call to displayResults.

Code: 
/* This program uses pointers to read into an array from a file, dynamically
    allocate a second array to fit the data read exactly, and fill the new array
    with the reverse of the original array entirely using pointer arithmetic. */

const int MAX_SIZE = 51;

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

int getData( FILE * is, const int MAX_SIZE, int * );
void allocateArray( int *, int * numberRead );
void displayResults( FILE * os, int *, int *, int * numberRead );

int main()
{
	int arrayForward[MAX_SIZE];
	int * arrayForPtr;
	arrayForPtr = arrayForward;
//	int *arrayBackward;
	
	FILE * fin;
	fin = fopen( "array.txt", "r" );
	if ( fin == NULL )
	{
		perror( "Error opening input file\n" );
//		system( "pause" ); only for Dev C++
		exit(1);
	}

	//get data
	int numberRead = 0;
	numberRead = getData( fin, MAX_SIZE, arrayForPtr );
	
	fclose( fin );
	
	allocateArray( arrayForPtr, &numberRead );
	
//	displayResults( stdout, arrayForward, arrayBackward, &numberRead );
	
	return 0;
}

int getData( FILE * is, const int MAX_SIZE, int * arrFor )
{
	int nu = 0;
	int used = 0;
	int * arrayEnd = arrFor + (MAX_SIZE - 1);
	
	while ( arrFor < arrayEnd && fscanf( is, "%i", arrFor ) == 1 )
	{
		++arrFor;
//		printf("%4i\n", arrFor[nu - 1]);
		used++;
	}
	
	//temporary print to see the data read
/*	int u;
	for ( u = 0 ; u < nu ; u++ )
	{
		printf("%4i\n", arrFor[u]);
		
	} */
	
	return used;
}

void allocateArray( int * arrFor, int * numberRead )
{
	int * arrBack;
	arrBack = (int *)calloc( *numberRead, sizeof(int) ); //allocate array
//	arrayBackPtr = arrayBack;
	int * arrayEnd = arrFor + (*numberRead); //find bounds
//	int * end;
//		end = &arrayEnd;
 
	//fill new array with contents of first in reverse order
	
	//use subscripts
/*	int u = 0, count = *numberRead;
	while ( u < *numberRead )
	{
		if ( count > 0 )
		{
			arrayBack[count - 1] = arrFor[u];
			count--;
		}
		u++;
	} */
	
	//use pointer arithmetic
	while ( arrFor < arrayEnd )
	{
		if ( arrayEnd + 1 > arrFor )
		{
			*(arrBack - 1) = *arrFor;
			arrBack--;
		}
		arrFor++;
	}
	
        //possibly faulty call to displayResults
	displayResults( stdout, arrFor, arrBack, numberRead );
 
	//temporary print
	
/*	printf("\n");
	for ( int count = 0 ; count < *numberRead ; count++ )
	{
		printf("%4i\n", arrBack[count]);
	} */

} 

void displayResults( FILE * os, int * arrFor, int * arrBack, int * numberRead )
{
	int * arrayEndFor = arrFor + (*numberRead); //bounds for original array
	int * arrayEndBack = arrBack + (*numberRead); //bounds for reversed array
	
	fprintf( os, "%24s\n\n%14s%17s\n", "The Arrays", "Original", "Reverse" );
	
	int nu = 0;
/*	while ( nu  < *numberRead )
	{
		printf("%4i\n", arrFor[nu] );
		nu++;
	} */
	while ( arrFor < arrayEndFor && arrBack < arrayEndBack )
	{
		fprintf( os, "%14i%17i\n", *arrFor, *arrBack );
		arrFor++, arrBack++;
	} 
        free(  arrBack ); 

}

input file (array.txt):

Code: 
62
55
83
90
97
104
107
148
156
160
186
190
209
217
226
232
240
283
291
295

output (to stdout):

Code: 
 The Arrays

      Original          Reverse
    1606419960              295
         32767              291
             7              283
             0              240
    1606664232              232
         32767              226
    1606416440              217
         32767              209
    1606416384              190
         32767              186
    1606424274              160
         32767              156
             0              148
             0              107
    1606418464              104
         32767               97
             0               90
             0               83
    1606416424               55
         32767               62
 
Last edited:
M

mac2x

macrumors 65816
Original poster
Sep 19, 2009
1,146
0
Please note that anyone who has read the OP as of this post has NOT seen the latest version of the code. I fixed a few flaws (like not freeing the dynamically allocated memory). However, the output is still partially junk.
 
lee1210

lee1210

macrumors 68040
Jan 10, 2005
3,182
3
Dallas, TX
mac2x said:
Please note that anyone who has read the OP as of this post has NOT seen the latest version of the code. I fixed a few flaws (like not freeing the dynamically allocated memory). However, the output is still partially junk.
Click to expand...

Then post the latest so we aren't chasing specters.

-Lee
 
M

mac2x

macrumors 65816
Original poster
Sep 19, 2009
1,146
0
I did. When I said I fixed them, I meant exactly what I said. I fixed them, both here and in the source file. :)
 
chown33

chown33

Moderator
Staff member
Aug 9, 2009
10,750
8,422
A sea of green
Add the code hilited in red:
Code: 
/* This program uses pointers to read into an array from a file, dynamically
    allocate a second array to fit the data read exactly, and fill the new array
    with the reverse of the original array entirely using pointer arithmetic. */

const int MAX_SIZE = 51;

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

int getData( FILE * is, const int MAX_SIZE, int * );
void allocateArray( int *, int * numberRead );
void displayResults( FILE * os, int *, int *, int * numberRead );

[COLOR="Red"]static void
showArray( int * array, int len )
{
	printf( "--- len: %d ---\n", len );
	while ( len-- > 0 )
	{  printf( "%4d\n", *array++ );  }
}
[/COLOR]

int main()
{
	int arrayForward[MAX_SIZE];
	int * arrayForPtr;
	arrayForPtr = arrayForward;
//	int *arrayBackward;
	
	FILE * fin;
	fin = fopen( "array.txt", "r" );
	if ( fin == NULL )
	{
		perror( "Error opening input file\n" );
//		system( "pause" ); only for Dev C++
		exit(1);
	}

	//get data
	int numberRead = 0;
	numberRead = getData( fin, MAX_SIZE, arrayForPtr );
	
	fclose( fin );
	
[COLOR="red"]	showArray( arrayForPtr, numberRead );
[/COLOR]
	allocateArray( arrayForPtr, &numberRead );
	
//	displayResults( stdout, arrayForward, arrayBackward, &numberRead );
	
[COLOR="red"]	showArray( arrayForPtr, numberRead );
[/COLOR]
	return 0;
}
Sample output:
Code: 
--- len: 20 ---
  62
  55
  83
  90
  97
 104
 107
 148
 156
 160
 186
 190
 209
 217
 226
 232
 240
 283
 291
 295
              The Arrays

      Original          Reverse
   -1610547840 :               295
   -1073743601 :               291
   -1073743640 :               283
   -1879043646 :               240
   -1610547840 :               232
         12314 :               226
   -1880799136 :               217
   -1879044454 :               209
   -1879044454 :               190
             0 :               186
   -1073743576 :               160
   -1879044387 :               156
   -1610547840 :               148
   -1610547588 :               107
             1 :               104
   -1073743392 :                97
   -1073743300 :                90
   -1073743400 :                83
   -1073743504 :                55
   -1881067089 :                62
--- len: 20 ---
  62
  55
  83
  90
  97
 104
 107
 148
 156
 160
 186
 190
 209
 217
 226
 232
 240
 283
 291
 295
Clearly, the original array is not corrupted. Which means something else is wrong.

I suggest adding additional calls to showArray() at strategic points in other functions, so you can see what happens. I've already found the problem, but it's better that you gain the debugging experience so I'm not going to give the answer yet.

Also, you have some very weird design choices, like passing numberRead by its address instead of by value. There's no reason the pass by address, and it makes certain things clumsy or error prone. That's not the error, but it's one waiting to happen.

You also have a redundant test in this code:
Code: 
	//use pointer arithmetic
	while ( arrFor < arrayEnd )
	{
		if ( arrayEnd + 1 > arrFor )
		{
			*(arrBack - 1) = *arrFor;
			arrBack--;
		}
		arrFor++;
	}
The 'while' condition has already been evaluated by the time the 'if' condition is evaluated. Since the one is subsumed by the other, there is no purpose for the if: i.e. it can never happen.

There are other oddness issues, but you need to find the bug first.
 
W

willieva

macrumors 6502
Mar 12, 2010
274
0
You make the call to displayResults( stdout, arrFor,...) right after a loop where you incremented the arrFor pointer. arrFor is no longer pointing at the beginning of the array.
 
S

SidBala

macrumors 6502a
Jun 27, 2010
533
0
You are decrementing off arrBack in allocate Array.

Code: 
	//use pointer arithmetic
	while ( arrFor < arrayEnd )
	{
		if ( arrayEnd + 1 > arrFor )
		{
			*(arrBack - 1) = *arrFor;
			arrBack--;
		}
		arrFor++;
	}

arrBack is pointing to the start of the backwards array and not the end of it!

So it is running into garbage memory and you are not really assigning anything into the real backwards array. It is getting assigned into a memory location in front of the backwards array.
 
Last edited:
S

SidBala

macrumors 6502a
Jun 27, 2010
533
0
Also in allocarray:

Code: 
        //possibly faulty call to displayResults
	displayResults( stdout, arrFor, arrBack, numberRead );

you can't do that since you have modified arrFor. arrFor is now pointing to the end of the forwards array and not to the start. Hence it is corrupted since there is nothing stored beyond the end of the forwards array.

If you made the change I suggested in the previous post, then arrBack should be pointing in the correct place.



PS: When you do arrFor ++, you are changing arrFor permanently. So when you want to do something again with arrFor, you need to get it back to the original location it was pointing to. So use a "position" or something as the pointer you increment and decrement:

Code: 
int * forPos = arrFor;
int * backPos = arrBack's last element;


while(forPos < the end of arrFor)
{

*backPos = *forPos;

backPos --;
forPos ++;
}

display(arrFor,arrBack)
free(arrFor);
free(arrBack);
 
Last edited:
M

mac2x

macrumors 65816
Original poster
Sep 19, 2009
1,146
0
Thanks for the pointers (pun intended!!). I made a few fundamental changes, such as returning the pointer to my backwards array to main and making the call to displayResults() in main rather than in allocateArray(), which was making things way too convoluted.

chown33: I would be interested to hear what is weird.....I feel this revision has fixed some of that though. BTW, your extra code caused a segfault; I added it exactly as indicated. I spotted the issue without it anyway (no I hadn't seen willieva's post yet).

Code: 
/* This program uses pointers to read into an array from a file, dynamically
 allocate a second array to fit the data read exactly, and fill the new array
 with the reverse of the original array entirely using pointer arithmetic. */

const int MAX_SIZE = 51;

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

int getData( FILE * is, const int MAX_SIZE, int * );
int * allocateArray( int *, int numberRead );
void displayResults( FILE * os, int *, int *, int numberRead );

int main()
{
	int arrayForward[MAX_SIZE];
	int * arrayForPtr = 0;
	
	FILE * fin;
	fin = fopen( "pointer.txt", "r" );
	if ( fin == NULL )
	{
		perror( "Error opening input file\n" );
//		system( "pause" ); only for Dev C++
		exit(1);
	}

	//get data
	int numberRead = 0;
	numberRead = getData( fin, MAX_SIZE, arrayForward );
	
	fclose( fin );
	
	int * arrayBackward = allocateArray( arrayForward, numberRead );
	
	displayResults( stdout, arrayForward, arrayBackward, numberRead );
	
	return 0;
}

/* This function takes the parameters: is (type file pointer), const int 
 * MAX_SIZE (51), and arrFor (pointer to an array of ints).
 */


int getData( FILE * is, const int MAX_SIZE, int * arrFor )
{
	int nu = 0;
	int used = 0;
	int * arrayEnd = arrFor + (MAX_SIZE - 1);
	
	while ( arrFor < arrayEnd && fscanf( is, "%i", arrFor ) == 1 )
	{
		++arrFor;
		used++;
	}
		
	return used;
}

/* This function takes the parameters: arrFor (pointer to an array of ints),
 * and numberRead (pointer to an int).
 */

int * allocateArray( int * arrFor, int numberRead )
{
	int * arrBack;
	arrBack = (int *)calloc( numberRead, sizeof(int) ); //allocate array
	int * arrayEnd = arrFor + ( numberRead ); //find bounds
 
	//fill new array with contents of first in reverse order
	
	//using subscripts
/*	int u = 0, count = *numberRead;
	while ( u < *numberRead )
	{
		if ( count > 0 )
		{
			arrayBack[count - 1] = arrFor[u];
			count--;
		}
		u++;
	} */
	
	//using pointer arithmetic
	while ( arrFor < arrayEnd )
	{
                *(arrBack - 1) = *arrFor;
		arrBack--;
		arrFor++;
	}
	
	return arrBack;
} 

/* This function takes the parameters: os (type file pointer), arrFor 
 * (pointer to original array of ints), arrBack (pointer to reversed
 * array of ints), and numberRead (pointer to int).
 */

void displayResults( FILE * os, int * arrFor, int * arrBack, int numberRead )
{
	int * arrayEndFor = arrFor + (numberRead); //bounds for original array
	int * arrayEndBack = arrBack + (numberRead); //bounds for reversed array
	
	fprintf( os, "%24s\n\n%14s%17s\n", "The Arrays", "Original", "Reverse" );
	
	while ( arrFor < arrayEndFor && arrBack < arrayEndBack )
	{
		fprintf( os, "%14i%17i\n", *arrFor, *arrBack );
		arrFor++, arrBack++;
	} 
	
	free( arrBack ); //free dynamically allocated memory
}
 
chown33

chown33

Moderator
Staff member
Aug 9, 2009
10,750
8,422
A sea of green
mac2x said:
chown33: I would be interested to hear what is weird.....I feel this revision has fixed some of that though.
Click to expand...
They're mostly fixed.

You still have a bug here:
Code: 
	free( arrBack ); //free dynamically allocated memory
Look at what comes before that, and tell us whether it's freeing the pointer that was allocated, or is it freeing some other pointer value.


BTW, your extra code caused a segfault; I added it exactly as indicated. I spotted the issue without it anyway (no I hadn't seen willieva's post yet).
Click to expand...
You'll have to post the complete compilable code that causes a segfault. It doesn't fail here, compiled for i386 or x86_64.
 
M

mac2x

macrumors 65816
Original poster
Sep 19, 2009
1,146
0
chown33 said:
They're mostly fixed.

You still have a bug here:
Code: 
	free( arrBack ); //free dynamically allocated memory
Look at what comes before that, and tell us whether it's freeing the pointer that was allocated, or is it freeing some other pointer value.
Click to expand...

I was wondering about that. valgrind indicated that the #allocs != #frees. Let me think about how to solve this one. :)


You'll have to post the complete compilable code that causes a segfault. It doesn't fail here, compiled for i386 or x86_64.
Click to expand...

I will worry about that another time, since I fixed most my own problems.
 
gnasher729

gnasher729

Suspended
Nov 25, 2005
17,980
5,565
mac2x said:
Code: 
void displayResults( FILE * os, int * arrFor, int * arrBack, int numberRead )
{
	int * arrayEndFor = arrFor + (numberRead); //bounds for original array
	int * arrayEndBack = arrBack + (numberRead); //bounds for reversed array
	
	fprintf( os, "%24s\n\n%14s%17s\n", "The Arrays", "Original", "Reverse" );
	
	while ( arrFor < arrayEndFor && arrBack < arrayEndBack )
	{
		fprintf( os, "%14i%17i\n", *arrFor, *arrBack );
		arrFor++, arrBack++;
	} 
	
	free( arrBack ); //free dynamically allocated memory
}
Click to expand...

Just as an example, I would have written this function as

Code: 
void displayResults (FILE* os, int arrForward [], int arrBackward [], int numberRead )
{
	fprintf( os, "%24s\n\n%14s%17s\n", "The Arrays", "Original", "Reverse" );
	for (int i = 0; i < numberRead; ++i)
		fprintf (os, "%14i%17i\n", arrForward [i], arrBackward [i]);
	
	free (arrBackward);
}

Lots shorter, lots more readable, and calls free () for the right pointer. Except that this function shouldn't call free (arrBack); I should be able to call a function called "displayResults" once, twice, or not at all, whatever I like, and I can't the way it is written because it calls free () for the "arrBack" array
 
M

mac2x

macrumors 65816
Original poster
Sep 19, 2009
1,146
0
Thanks to valgrind, %p, and much manual work with the addresses, I think I finally arrived at a much improved program logically.

@gnasher: I would do that, except for the fact that I want to use only pointer arithmetic in this program. ;) Otherwise, I agree with you 100%. ;)

Code: 
/* This program uses pointers to read into an array from a file, dynamically
 allocate a second array to fit the data read exactly, and fill the new array
 with the reverse of the original array entirely using pointer arithmetic. */

const int MAX_SIZE = 51;

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

int getData( FILE * is, const int MAX_SIZE, int * );
int * allocateArray( int *, int numberRead );
void displayResults( FILE * os, int *, int *, int numberRead );

int main()
{
	int arrayForward[MAX_SIZE];
	int * arrayForPtr = 0;
	
	FILE * fin;
	fin = fopen( "pointer.txt", "r" );
	if ( fin == NULL )
	{
		perror( "Error opening input file\n" );
//		system( "pause" ); only for Dev C++
		exit(1);
	}

	//get data
	int numberRead = 0;
	numberRead = getData( fin, MAX_SIZE, arrayForward );
	
	fclose( fin );
	
	int * arrayBackward = allocateArray( arrayForward, numberRead );
	
	displayResults( stdout, arrayForward, arrayBackward, numberRead );
	
	free( arrayBackward ); //free dynamically allocated memory
	
	return 0;
}

/* This function takes the parameters: is (type file pointer), const int 
 * MAX_SIZE (51), and arrFor (pointer to an array of ints).
 */


int getData( FILE * is, const int MAX_SIZE, int * arrFor )
{
	int nu = 0;
	int used = 0;
	int * arrayEnd = arrFor + (MAX_SIZE - 1);
	
	while ( arrFor < arrayEnd && fscanf( is, "%i", arrFor ) == 1 )
	{
		++arrFor;
		used++;
	}
		
	return used;
}

/* This function takes the parameters: arrFor (pointer to an array of ints),
 * and int numberRead.
 */

int * allocateArray( int * arrFor, int numberRead )
{
	int * arrBack;
	arrBack = (int *)calloc( numberRead, sizeof(int) ); //allocate array
	int * arrayEnd = arrFor + ( numberRead - 1 ); //find bounds
	int * arrBackPtr = arrBack + (numberRead - 1 ); //bounds for reversed array
	
	//fill new array with contents of first in reverse order
	while ( arrFor <= arrayEnd )
	{
		*(arrBackPtr) = *arrFor;
		arrFor++;
		arrBackPtr--;
	}
		
	return arrBack;
} 

/* This function takes the parameters: os (type file pointer), arrFor 
 * (pointer to original array of ints), arrBack (pointer to reversed
 * array of ints), and int numberRead.
 */

void displayResults( FILE * os, int * arrFor, int * arrBack, int numberRead )
{
	int * arrayEndFor = arrFor + (numberRead); //bounds for original array
	int * arrayEndBack = arrBack + (numberRead); //bounds for reversed array
	
	fprintf( os, "%24s\n\n%14s%17s\n", "The Arrays", "Original", "Reverse" );
	
	while ( arrFor < arrayEndFor && arrBack < arrayEndBack )
	{
		fprintf( os, "%14i%17i\n", *arrFor, *arrBack );
		arrFor++, arrBack++;
	} 
	
	arrBack -= numberRead;
}
 
S

Sander

macrumors 6502a
Apr 24, 2008
521
67
gnasher729 said:
Except that this function shouldn't call free (arrBack); I should be able to call a function called "displayResults" once, twice, or not at all, whatever I like, and I can't the way it is written because it calls free () for the "arrBack" array
Click to expand...

This, in bold-faced red flashing letters please.
 
lee1210

lee1210

macrumors 68040
Jan 10, 2005
3,182
3
Dallas, TX
You don't need to "fix" the pointer in displayResults now, since you moved the free and it's passed in by value.

Also, if the purpose of this is pointer manipulation, why store a reversed copy at all? The logic in displayResults would be exactly the same except one pointer would walk backwards. Saves on storage and eliminates the pesky calloc/free.

-Lee
 
You must log in or register to reply here.
Register on MacRumors! This sidebar will go away, and you'll see fewer ads.
Top Bottom