Array loses data?

[farmerdoug]

Data is read from a file into an array.
031
032
033
035
036
037
039
040
Then read out.
0 040
1 040
2 040
3 040
4 040
5 040
6 040

What's going on. A silly mistake I would think but I can't find it.

#include "string.h"
#include "stdlib.h"
#include "stdio.h"
#include "pcxp.h"
#include "math.h"
#define location_of_images "/users/doug/kappa_and/slices/"
#define number_of_cubes 8
#define number_of_slices 32


int main(int argc, char *argv[])
{
	int cube, slice, row, col, inner_radius, outer_radius, x_off, y_off, row_off, col_off, best_inner_radius, best_outer_radius;
	fitsfile *fptr;
	
	int status = 0;
	int  anynull;
	float nullval;
	float *dbuf1, *dbuf2;
	float ***centers;
	
	char *slice1, *slice2, *image_directory, *tmp, *cube_number, **cube_numbers, *image1, *image2;
	float min = 1e10, diff, radius;
	FILE *x_ctrs, *y_ctrs, *cube_list;
	fptr = (fitsfile*)calloc(1, sizeof(fitsfile));
	

	dbuf1 = (float*) calloc(large_image_size, sizeof(float));
	dbuf2 = (float*) calloc(large_image_size, sizeof(float));
	
	centers = (float ***)calloc(number_of_cubes, sizeof(float**));
		for (cube = 0; cube < number_of_cubes; cube++)
			centers[cube] = (float**) calloc(number_of_slices,sizeof(float*));
		for (cube = 0; cube < number_of_cubes; cube++)
			for(slice = 0; slice < number_of_slices; slice++)
				centers[cube][slice] = (float *) calloc(2, sizeof(float));
	tmp = (char*) calloc(40, sizeof(char));
	cube_number = (char*) calloc(40, sizeof(char));
	cube_numbers = (char**) calloc(number_of_cubes, sizeof(char*));
	for (cube = 0; cube < number_of_cubes; cube++)
		cube_numbers[cube] = (char*) calloc(5,sizeof(char));
	image1 = (char*) calloc(100, sizeof(char));
	image2 = (char*) calloc(100, sizeof(char));
	image_directory = (char*) calloc(100, sizeof(char));
	strcpy(image_directory, location_of_images);
	
	
	
	if ((x_ctrs = fopen("/users/doug/kappa_and/HD222439_2012-12-23CenterX1.dat","r")) == NULL)
		printf("didn't open X file\n");
		
	if ((y_ctrs = fopen("/users/doug/kappa_and/HD222439_2012-12-23CenterY1.dat","r")) == NULL)
		printf("didn't open Y file\n");
	
	for (cube = 0; cube < number_of_cubes; cube++)
		for (slice = 0; slice < number_of_slices; slice++)
			{
			fgets(tmp, 40, x_ctrs);
			centers[cube][slice][0] = atof(tmp);
			
			fgets(tmp, 40, y_ctrs);
			centers[cube][slice][1] = atof(tmp);
			
			//printf("%d %d %6.2f %f6.2\n",cube, slice,  centers[cube][slice][0],centers[cube][slice][1]);
			}
	fclose(x_ctrs);
	fclose(y_ctrs);
	
	
	if ((cube_list = fopen("/users/doug/kappa_and/slices/cube_list.txt","r")) == NULL)
		printf("didn't open cube list\n");
	cube= 0;
		while(fscanf(cube_list, "%s\n",cube_number) != EOF)
			{
			cube_numbers[cube]= cube_number;
			printf("%s\n", cube_numbers[cube]);
			cube++;
			
			}
// alignment for each slice
//for ( slice = 0; slice < number_of_slices; slice++)
	for (cube = 0; cube < number_of_cubes - 1; cube++){	
	printf("%d %s\n",cube, cube_numbers[cube]);
		}


return(1640);
}

[ElectricSheep]

	cube_number = (char*) calloc(40, sizeof(char));
	cube_numbers = (char**) calloc(number_of_cubes, sizeof(char*));

You have defined cube_number as a pointer to the first element in a series of forty character types.

You have defined cube_nubmers as a pointer to the first element in a series of number_of_cubes pointers to character types.

	cube= 0;
	while(fscanf(cube_list, "%s\n",cube_number) != EOF)
	{
		cube_numbers[cube]= cube_number;
		printf("%s\n", cube_numbers[cube]);
		cube++;	
	}

fscanf() is parsing a string-type into the location specified by cube_number. But, because you are not changing cube_number via any kind of pointer-arithmetic, each iteration will load the next string-type into the same location, overwriting the previous data you put there.

You need to increment cube_number along with cube.

[farmerdoug]

cube_number is a string for the storage of numbers coming out of the file
cube_numbers is a 2-d array for the storage of (cube_number)s.
The while loop reads the file; puts the numbers in the array and outputs the array. The for loop is supposed to output the numbers in the array but doesn't.

[ElectricSheep]

I'm going to repeat this:

because you are not changing cube_number via any kind of pointer-arithmetic, each iteration will load the next string-type into the same location

cube_number is a pointer. Its value is a number representing a location in memory. Since you do not change cube_number with each iteration in your fscanf() loop, you are loading data into the same location in memory and overwriting the data you loaded previously.

If you were look at the execution of your program in a debugger and inspect the contents of cube_numbers, you would find that you have put the exact same pointer address into every single member of the array.

[farmerdoug]

In the while loop, I load a string in char* cube_number from the file into cube_number then write it to the array cube_numbers. I then print out the what I have put into the array before incrementing the index for cube_numbers. The print statement and its output shows that the array has been properly loaded.

Then the same printf statement in the for loop calls the values in cube_numbers but doesn't print out the same values.
Sorry I don't see what you are talking about.

----------

Got it. of, course.

I'm dealing with strings.

not

cube_numbers[cube] = cube_number

strcpy(cube_numbers[cube],cube_number);

thanks.

[ElectricSheep]

Originally Posted by farmerdoug

[/COLOR]Got it. of, course.

I'm dealing with strings.

not

cube_numbers[cube] = cube_number

strcpy(cube_numbers[cube],cube_number);

thanks.

srtcpy() will not allocate new space for the copy, you need to explicitly do that. I would expect that if you try to just use strcpy() to copy directly into your cube_numbers[] array, you will crash when attempting to dereference a NULL pointer. Your loop should really look more like this:

        char *input_buf = (char *) calloc(40, sizeof(char));
        cube= 0;
	while(fscanf(cube_list, "%s\n",input_buf) != EOF)
	{
		// Allocate space for the string, and copy it
                cube_number = (char *)malloc(40, sizeof(char));
                strlcpy( cube_number, input_buf, 40 * sizeof(char));

                cube_numbers[cube]= cube_number;
		printf("%s\n", cube_numbers[cube]);
		cube++;	
	}

[farmerdoug]

worked just fine.
There is plenty of space allocated.

cube_numbers = (char**) calloc(number_of_cubes, sizeof(char*));
for (cube = 0; cube < number_of_cubes; cube++)
cube_numbers[cube] = (char*) calloc(5,sizeof(char));

[gnasher729]

Originally Posted by farmerdoug

			cube_numbers[cube]= cube_number;

That's the problem. cube_number is a single char* with space for 39 chars + trailing zero byte. This line stores the same pointer into each element of cube_numbers. cube_numbers [0], cube_numbers [1] etc. are all the same pointer when that loop is finished.

I suppose you wanted to write

			strcpy (cube_numbers[cube], cube_number);

And I suppose you wanted to allocate more than 5 bytes for each of the strings in cube_numbers.

[farmerdoug]

I figured out that I needed strcpy about 3 or 4 posts ago. The 5 chars are enough because I wrote the file myself.

thanks.

[robvas]

Doug - your code formatting style (or lack thereof) is very, very strange. Also why hard-code filenames into the code and not accept them as commandline arguments?

[farmerdoug]

I do what I need to do to get the job done as quickly as possible. For the foreseeable future these are the only files I will need. If and when this work sees the light of day, there will be no files; the data will come from arrays.

If you think this style is bad, you should have seen it before.