0

I'm trying to read some float values as an array to a struct member as shown below:

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

#define MAX_IN 10

typedef struct S_t S_t;

struct S_t {
    float *sptr; 
    uint32_t ns;
};


S_t *getInputS(char *sdfile) 
{
FILE *inSFP;
float fvls[MAX_IN];
static S_t inS;
int n;

inSFP = fopen(sdfile, "r");
if (inSFP == NULL) {
    printf("\nFailed to open input file...!!!\n");
}
else {
    n = 0;
    while (fscanf(inSFP, "%f", &fvls[n]) != EOF) {
    printf("fvls[%d] = %f\n", n, fvls[n]);
    n++;
    }
    printf("\nScanned all inputs....\n");
    inS.ns = (uint32_t) n;
    inS.sptr = (float *) malloc(n * sizeof(float));
    inS.sptr = fvls;
    for(int i = 0; i < n; i++)
    printf("inS.sptr[%d] = %f\n", i, inS.sptr[i]);

    printf("\nInput read from file %s....\n", sdfile);

    fclose(inSFP);
    printf("\nClosed file...");
}
return &inS;
}


int main(int argc, char *argv[]) 
{
S_t *inpS = malloc(sizeof(*inpS));
inpS->sptr = malloc(MAX_IN * sizeof(inpS->sptr));
S_t *outS = malloc(sizeof(*outS));
outS->sptr = malloc(MAX_IN * sizeof(outS->sptr));
static uint32_t n;

char *inFN = argv[1];
char *outFN = argv[2];

inpS = getInputS(inFN);
printf("\nContent from main : \n");
n = inpS->ns;
for(int i = 0; i < n; i++)
    printf("%f", *(inpS->sptr + i));
    // printf("%f", inpS->sptr[i]);
printf("\nS structure updated (ns = %d)....\n", n);

return 0;
}

This returns the following:

fvls[0] = 0.430000
fvls[1] = 0.563210
fvls[2] = 0.110000
fvls[3] = 1.230000
fvls[4] = -0.034000

Scanned all inputs....
inS.sptr[0] = 0.430000
inS.sptr[1] = 0.563210
inS.sptr[2] = 0.110000
inS.sptr[3] = 1.230000
inS.sptr[4] = -0.034000

Input read from file in.txt....

Closed file...
Content from main :
-0.0000000.0000000.0000000.000000-nan
S structure updated (ns = 5)....

Input values (Original Input):
[0.000000, 0.000000, -0.000000, 0.000000, -0.000000]

The values are indeed read from the input file by the function getInputS() correctly, but on return the member sptr's values are returned incorrectly. I am using a static variable of type S_t to store the values so that the value is retained. Inspite of that, the values seem to have lost! What am I doing wrong here? How do I fix this?

5
  • Can you post a sample of the input file? Commented Jul 3, 2022 at 22:32
  • It's a simple text file with values 0.43 0.56321 0.11 1.23 -0.034. Commented Jul 3, 2022 at 22:33
  • inS.sptr = fvls; what happens to fvls when you exit the function scope? Commented Jul 3, 2022 at 22:34
  • 2
    You have allocated memory, but you haven't copied any data, instead, you overwrite the return value of malloc. Commented Jul 3, 2022 at 22:36
  • fvals is kind of temporary variable that is destroyed when the function is exited. I just wanted it for counting the array numbers. Commented Jul 3, 2022 at 22:46

1 Answer 1

3

In this line:

inS.sptr = (float *) malloc(n * sizeof(float));

you cause sptr to point to a malloc'ed region of just the right size for your inputs. That's good. But in the very next line:

inS.sptr = fvls;

you throw the malloc'ed pointer away, overwriting it with a pointer to the local fvls array, which is going to disappear when this function returns.

Stated another way: when you store a pointer in a pointer variable, it's just like storing any other value in any other variable. Any time you do something like this:

a = b;
a = c;

you are throwing away the effect of the first assignment, and when you're done, all a is left holding is the value of c. It's no different when a is a pointer variable and b and c are two different pointer values.

Whenever you work with pointers, you have to keep clear in your mind the distinction between the pointer and what the pointer points to. In your getInputS function, you have to worry about both: you have to set the pointer sptr to point to valid storage to contain the values you've read, and you have to set what the pointer points to to be those values. When you say

inS.sptr = (float *) malloc(n * sizeof(float));

you're setting the pointer. But when you say

inS.sptr = fvls;

you're resetting the pointer to something else. But you never get around to setting what the pointer points to (the first, malloc'ed pointer, that is) at all.

And, to be clear, the line

inS.sptr = fvls;

copies the pointer, it does not copy the pointed-to data, which is what you need at this point.

To fix this, either:

  1. Copy data from fvls to sptr (in place of the line inS.sptr = fvls). You can call memcpy, or you can use a loop repeatedly assigning sptr[i] = fvls[i].
  2. Get rid of fvls, initialize sptr to malloc(MAX_IN * sizeof(float));, read directly into sptr, and then at the end, if you want to try to minimize wasted space, call realloc to reallocate sptr down to just big enough for the number of values you actually read.
Sign up to request clarification or add additional context in comments.

5 Comments

Okay. Understood your 2nd suggestion. Could you explain "copying" here?
@skrowten_hermit Copy each float from fvls to sptr. Either iterate and assign each element of fvls to sptr or use a function like memcpy or memmove.
@skrowten_hermit Also, note that, in main, you allocate inpS and its members. But, you throw away this with: inpS = getInputS(inFN); [a memory leak]. And, the return value of getInputS is a static, so less flexible. I'd either pass main's pointer to the function or have the function do all the allocation (i.e. eliminate the static variable).
@skrowten_hermit See updated answer, or alex01011's comment.
@CraigEstey Yours is a valuable input. I have made the modifications you have suggested too.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.