CODEWITHSUNDEEP
carraysstring
zappa
zappa

Reputation: 336

Pass Strings in C and fill them

So I'm trying to open a textfile, which looks something like this:

name 1.52
someothername 2.42
...and so on

Now I have to copy these strings and floats into arrays, so I can mess around with them later in my code. The thing is, I'm having trouble assigning the strings that are being found in the textfile my array that I need somewhere else in my code. The arrays have the space 100, because this is supposed to be the maximum length that I allow. I know that the problem lies here

char a[100];

and here

fscanf(fp, "%s %f\n", &a, &b);

how can I pass my empty array to the fscanf function properly so that it can be "filled"??
I keep getting errors like
"warning: format specifies type 'char ' but the argument has type 'char ()[100]' [-Wformat]"

/* reads the list into the arrays name and prices and returns the number of lines (i.e.
elements) that have been read */

int read_priceList(const char* path, char names[100][100], 
float price[100]){
FILE *fp = fopen(<path>, "r");

char a[100];
float b;
int i = 0;

if(fp == NULL) {
    perror("Error opening File");
    return -1;
}
while (!feof(fp) && i < 100) {
    fscanf(fp, "%s %f\n", &a, &b);
    strcpy(names[i], &a);
    price [i] = b;
    i++;
}
i--;
fclose(fp);
return i;
}

Also later in my code I want to pass an empty, two dimensional string array to this function, which is also a problem...
warning: incompatible pointer types passing 'char ' to parameter of type 'char ()[100]
First, I initialize them (this now happens in the main function, which calls the read_pricelist function)

char namesPricelist[100][100];
float prices[100];

and then I want to call the readPricelist function and give them these parameters.

readPricelist(<path>, &namespricelist[0][0], &prices[0]);

I'm starting to lose my mind, please help...

Upvotes: 0

Views: 293

Answers (1)

WhozCraig
WhozCraig

Reputation: 66234

Among the things wrong with your function:

  1. Regarding the warning, you're passing a char (*)[100] as a formatted target expecting a target of char *. The value you should be passing is the base address of the Nth element needing to be populated.
  2. while (!feof(fp)...) is wrong Read this to see why.
  3. In conjunction with the above, you never validate the fscanf did, in fact, actually work. Rather, you assume it did and blindly throw indeterminate data in to your output buffer. This is especially dangerous when the loop is first entered. If the first fscanf fails to parse anything the array a[] is left utterly indeterminate and the ensuing strcpy will be searching for a terminator to stop, but have no valid string populated in a with which to do so. If you're (un)lucky it will fine a null byte 0x00 somewhere within the first 100 chars, but such behavior is completely undefined.
  4. You leave yourself open to a potential buffer overrun with your wide-open %s format. It should be length-limited.
  5. You needlessly read data into temporaries. fscanf can read properly formatted data directly in your output targets; there is no need for intermediate targets and copying.

Applying all of the above can be done while significantly reducing the code-load, and is worth considering:

int read_priceList(const char* path, char names[100][100], float price[100])
{
    FILE *fp = fopen(path, "r");
    int i = 0;

    if(fp == NULL)
    {
        perror("Error opening File");
        return -1;
    }
    while (i<100 && fscanf(fp, "%99s %f", names[i], price+i) == 2)
        ++i;

    fclose(fp);
    return i;
}

There is still more that can be done, but this should get you considerably further down the road.

Upvotes: 3

Related Questions