9

I’m learning functions/pointers, and having a bit of an issue. What I need to write is a C program with main() and two other functions.

Requirements:

  1. read_funct() must allocate enough memory using malloc() to store the data.

  2. Function prototype for read_funct must be:

    int read_funct(int *data_num, double **data_vals, char *filename)
    

How it’s meant to work:

  1. main() calls the first function: read_funct()

  2. read_num() reads binary data from a file. Two values have to be extracted: the no. of values (first 4 bytes), then the values themselves (8 bytes each, so contained in the next 8*no. of values). These correspond to data_num and data_vals. They have to be printed, the program then returns to main().

  3. main() performs operations to edit the data from the first file.

  4. main() calls the second function: write_funct(), which writes the edited data into a new file.

Where I am:

The first function reads data_num correctly, and reads/prints data_vals. This is all working properly. However, I’m trying to print these in main() to verify that I’m performing operations on the correct data, but I can’t get it working.

Note: I’m not trying to get it working with write_funct() at the moment, just taking it step-by-step.

Here’s my current code for main():

int read_funct(int *data_num, double **data_vals, char *filename);
int main()
{
  int data_num;
  double **data_vals;

  //Reads file using read_funct()
  read_funct(&data_num, data_vals, filename);

  //Check: print data_num
  printf("\nCheck No. of Values: %d\n", data_num);

  //Check: print data_vals
  for(int i = 0; i<data_num; i++)
  {
        printf("%0.3lf\t", data_vals[i]);
  }
  return(0);
}

Here’s read_funct()

int read_funct (int *data_num, double **data_vals, char *filename)
{
    FILE * fp = fopen(filename, "rb");                          //Opening file      
   //There's code here to check valid file open

   //There's code here to determine size and check valid length

  //Proceed with reading data_num if file is large enough
  char buffer_n[4];
  fread(buffer_n, 4, 1, fp);
  int res = buffer_n[0]|(buffer_n[1] << 8)|(buffer_n[2] << 16)|(buffer_n[3] << 24);     //Convert endian

  data_num = &res;          //Passes results back to data_num
  printf("Number of Data Values: %d \n", *data_num);    //Printing results

  //Proceeds with calculating data_vals
  data_vals = malloc(8*res);                //Allocating memory
  fread(data_vals, 8, res, fp); 

    //Prints data_vals
    for(int i=0; i<res; i++)
    {
        printf("%0.3f\t", data_vals[i]);
    }
    printf("\nEnd of File Read.\n\n");
    fclose(fp); 
    free(data_vals);                //Free allocated memory
    return(0);
}

Desired output:

Basically, I want it to print out the values from inside read_file() and then print a check in main(), so the output will be something like:

No. of values: 3            //From printf in read_file()
2 4 6

Check No. of values: 3      //From printf in main()
2 4 6

Where I think I'm going wrong:

  1. Fairly sure that the main issue is that I've messed up my pointers and how I've initialised things in main(). I've been trying to fix this by myself, but I think I need some more experienced help to figure this out.

  2. I know that every malloc() call must have a subsequent free(), but I'm worried that by doing so the way that I have, maybe I've made it so that I can't retrieve it in main(). Does it instead need to have an intermediate buffer to which memory is allocated instead?

Help to get this code working would be very greatly appreciated. Thank you!

2
  • 3
    Congratulations for a very well written question. Apr 9, 2017 at 6:31
  • Thank you - I figure that if I'm asking other people to help me out with something, the very least I can do is ensure that it's as easy to read as possible! Props also to Cool Guy, who made an edit to enlargen the header sections (rather than just the bold that I had). And thank you again for your help - your answer was perfect, very well laid-out and easy to follow.
    – ReyL
    Apr 9, 2017 at 8:02

2 Answers 2

6

Apart from freeing the data too soon, you have another problem here:

double **data_vals;
read_funct(&data_num, data_vals, filename);

If you want data_vals to be filled (written to, modified) by a function, you must pass its address, exactly as you do with data_num.

Here is another, slightly different, explanation. You see, you declare data_vals but you don't assign a value to it - it contains garbage. So it is a non-sense to pass data_vals to any function, or use it in any expression. It has a sense instead, to assign something to it, either via direct assignment or passing its address to a function, for the function to fill the variable.

Then, your usage of data_vals depicts a vector, or an array. So you really need to declare an array with [], or may be a pointer (pointers and arrays are quite related/interchangeable in C). The logic of your main() function requires a pointer, not a pointer to pointer. Hence, this is appropriate:

double *data_vals;

The function which writes to your pointer variable, instead, needs the address of the variable to write to; in other words: a pointer to a pointer. This is why your function has this (correct) signature:

read_funct(..., double **data_vals, ...)

To understand easily, let see the other (simpler) thing you wrote correctly:

int data_num;
read_funct(&data_num, ...);  // declaration: read_funct(int *data_num, ...)

You declare data_num as integer in main(); you declare read_funct() with a formal parameter of pointer to integer, then you call read_funct() passing the address of your variable data_num. Perfect. Now, do the same with the other variable, data_vals. You declare it as pointer to double, pass its address to read_funct() using the notation &data_vals, and your function read_funct() declares that parameter as a pointer to pointer to double (and writes to it using *data_vals = .... You can see the parallelism between the two variables, right?

May be I've been too pedantic, but your question was really clear and well formed, so I tried to do the same.

4
  • So perhaps suggesting a declaration of double *data_vals; and then read_funct(&data_num, &data_vals, filename);? (I'd also suggest declaring/opening the file in main and then making read_funct take a FILE *fp, e.g. read_funct(&data_num, &data_vals, FILE *fp); passing filename is fine, it's just not quite standard. Apr 9, 2017 at 6:47
  • Yes, I am suggesting that (I edited the answer meanwhile). And the idea of passing a FILE instead of a filename it's not too bad, but I would not say that it's "more standard". It depends a lot on the logic of the program. Apr 9, 2017 at 7:01
  • Thank you very, very much for your help, and your excellent answer. You did a fantastic job in explaining exactly where the problem was, why it was a problem, and how to correctly implement what I was trying to do (in a way not just limited to any singular problem). Major kudos!
    – ReyL
    Apr 9, 2017 at 7:59
  • @ReyL, the bottom line is to understand when you pass a variable to a function, the function receives a copy of the variable (in the case of a pointer the copy has its very own address -- separate and distinct from the pointer passed from the caller) To insure you are operating on the actual pointer address as it exists in main you want to pass the address of the pointer. (e.g. &ptr). That insures any changes to pointer in read_funct are reflected back in the calling function (main here). That's why declaring double *data_vals; and passing &data_vals is required. Apr 9, 2017 at 22:52
4

Yes, you are free-ing the buffer too soon. After you have freed it, there is not guarantee as to what it contains. You can free it at the end, in main.

1
  • 1
    I thought that would be the case. I've changed it, though the program still isn't returning the proper output. Thank you for the confirmation!
    – ReyL
    Apr 9, 2017 at 2:12

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.