0

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...

harrisonfooord
  • 308
  • 3
  • 19
  • 2
    Arrays should not have the `&` operator placed in front of them. The array name is the address of the array, so adding the address-of operator is redundant at best. – Carey Gregory Oct 22 '14 at 22:24
  • `%s` expects a `char *` but you're supplying a `char **`. Try with `fscanf(fp, "%s %f\n", a, &b);`. But you have another issue: %s is supposed to be null-terminated (0x00), but your string is space-terminated (0x20). So I doubt your `fscanf` will work at all. Have a look to `strtok()` function. – m-ric Oct 22 '14 at 22:26
  • @m-ric what "space-termianted' are you're talking about? `%s` will discard whitespace until non-whitespace or EOF is encountered, then proceed to fill until EOF or whitespace is encountered. Given the OP's sample data, the only thing needed to make this seemingly work is the first comment. the format specifiers are sufficient as-is. (There are plenty of other things wrong in the code beyond that). – WhozCraig Oct 22 '14 at 22:53
  • @WhozCraig I thought that `%s` needed a null-terminated string. But verified it doesn't. Thanks. When I posted my comment, @Carey Gregory's one was not visible... – m-ric Oct 23 '14 at 14:09

1 Answers1

3

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.

Community
  • 1
  • 1
WhozCraig
  • 65,258
  • 11
  • 75
  • 141