fscanf returning value of 0 - C

Question

I have a file that looks like this:

Kathryn 1561 2589.98

Hollie 2147 2496.36

Sherlock 3574 2514.65

and my fscanf - related code is as follows:

int load_data(char* fileName, char** accountNames, int* accountNumbers, float* amounts, int numOfAccounts)
{
    FILE *fPtr = fopen(fileName, "r");
    int counter = 1, test;

    if(fPtr  == NULL)
    {
            fclose(fPtr);
            return 0;
    }
    else
    {
            test = fscanf(fPtr,"%20s%d%f",  *accountNames, accountNumbers, amounts);
            printf("%d\n", test);
            while(counter < numOfAccounts)
            {
                test = fscanf(fPtr,"%20s%d%f",  *accountNames, accountNumbers, amounts);
                    printf("%d\n", test);
                    counter++;
            }

            fclose(fPtr);
            return 1;
    }

}

Here is the calling code:

int main(int argc, char** argv)
{
        int numOfAccounts = atoi(*(argv + 2));
        char* fileName = *(argv + 1);
        char** accountNames = malloc(sizeof(char) * 20 * numOfAccounts);
        int* accountNums = malloc(sizeof(int) * numOfAccounts);
        float* amounts = malloc(sizeof(float) * numOfAccounts);
        int dataload = load_data(fileName, accountNames, accountNums, amounts, numOfAccounts);
        dataload++;

return 0;
}

The if statement works, and the file is recognized. However, fscanf only returns 0. I'm really really new at C and fscanf so I ask that you be patient as I am learning, but if anyone could help me that would be great. Please let me know if I need to include more information. Thank you.

If you are really new at C, scanf might seem like a good idea. It is not. Read the file with fread or fgets or fgetc and parse the data yourself. Do not bother with scanf. — William Pursell, Nov 18 '16 at 16:51
You could check `ferror` and `feof` on `fPtr` and then check `errno` to see what exactly went wrong. — muXXmit2X, Nov 18 '16 at 16:54
Note that code is always scanning values to the same location. I'd expect something like `accountNumber + counter`. — chux - Reinstate Monica, Nov 18 '16 at 16:55
Should use `"%19s"` with `malloc(sizeof(char) * 20 * numOfAccounts)` — chux - Reinstate Monica, Nov 18 '16 at 16:56
@muXXmit2X when I use feof to terminate my loop it repeats indefinitely. — Not Steve, Nov 18 '16 at 16:57
Hmm I think `char** accountNames = malloc(sizeof(char) * 20 * numOfAccounts);` is a problem. `accountNames` is a pointer to pointers. — chux - Reinstate Monica, Nov 18 '16 at 17:01
@chux I wondered. The logic behind it is it's an array of strings, and each string is maximum 20 chars long, then we need as many strings as there are accounts. How can I fix it? — Not Steve, Nov 18 '16 at 17:03
I'd go for allocating `numOfAccounts` `char *` pointers into `accountNames` and then allocating a 20 `char` for each. The better approach would create a structure for `accountNames, accountNumbers, amounts`, but that may be beyond your current training. IAC, strongly suggest reading the line with `fgets()` as @William Pursell commented [above](http://stackoverflow.com/questions/40682160/fscanf-returning-value-of-0-c?noredirect=1#comment68594686_40682160). GTG — chux - Reinstate Monica, Nov 18 '16 at 17:06
@WilliamPursell when I use fgets(*accountNames, 20, fPtr) my program immediately gives me a segmentation fault. — Not Steve, Nov 18 '16 at 17:20
Change this line test = fscanf(fPtr,"%20s%d%f", *accountNames, accountNumbers, amounts); to this test = fscanf(fPtr,"%s %d %f", &(*accountNames), accountNumbers, amounts); — user4925, Nov 18 '16 at 17:46

score 1 · Answer 1 · answered Aug 12 '18 at 20:13

Your char **accountNames should be a 1D array, of length numOfAccounts, of type char *; and the memory allocation for accountNames is done in two steps, not one:

int anlen, ai;

accountNames = (char **) malloc (numOfAccounts * sizeof (char *));

anlen = 20;

for (ia=0; ia<numOfAccounts; ia++)
{
    numOfAccounts[ia] = (char *) malloc (anlen * sizeof (char));

}

Your counter can run from 0 to numOfAccounts rather than from 1 to numOfAccounts, so that you have only one fscanf statement, contained in your while (count<numOfAccounts) loop.

The string in your fscanf statement should be " %19s %d %f". Note the spaces here, meaning 'ignore any leading whitespace'. This is one of the features that makes fscanf such a powerful and useful function, when used properly. Also, your *accountNames in your fscanf statement should now be *(accountNames + count) or accountNames[count].

You may want to add a check that each name has been read, for example by inserting into your fscanf a %c to read the character immediately following the name on each line of the data file and checking this character using isspace(). If this shows an error, you might want to use, for example, the discussion here to enable fscanf to read names of more than 19 characters.

score 0 · Answer 2 · answered Nov 18 '16 at 18:09

I'm not advocating this approach but it seems to be what you're aiming for with your current code:

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

#define NAME_SIZE 20

int load_data(char *fileName, char *accountNames, int *accountNumbers, float *amounts, int numOfAccounts)
{
    FILE *fPtr = fopen(fileName, "r");

    if (fPtr == NULL)
    {
        return 0;
    }

    int count = 0;

    for (int counter = 0; counter < numOfAccounts; counter++)
    {
        if (fscanf(fPtr, "%19s %d %f",  accountNames + (counter * NAME_SIZE), accountNumbers + counter, amounts + counter) == 3)
        {
            count++;
        }
    }

    fclose(fPtr);

    return count;
}

void unload_data(char *accountNames, int *accountNumbers, float *amounts, int numOfAccounts)
{
    for (int counter = 0; counter < numOfAccounts; counter++)
    {
        printf("%s %d %8.2f\n",  &accountNames[counter * NAME_SIZE], accountNumbers[counter], amounts[counter]);
    }
}

int main(int argc, char *argv[])
{
    int numOfAccounts = atoi(argv[2]);
    char *fileName = argv[1];

    char *accountNames = calloc(numOfAccounts, NAME_SIZE);
    int *accountNums = calloc(numOfAccounts, sizeof(int));
    float *amounts = calloc(numOfAccounts, sizeof(float));

    int datacount = load_data(fileName, accountNames, accountNums, amounts, numOfAccounts);
    printf("%d\n", datacount);
    unload_data(accountNames, accountNums, amounts, datacount);

    free(accountNames);
    free(accountNums);
    free(amounts);

    return 0;
}

OUTPUT

% ./a.out test.dat 3
3
Kathryn 1561  2589.98
Hollie 2147  2496.36
Sherlock 3574  2514.65
%

Any time your code relies on parallel arrays, you're missing a proper data structure.

score 0 · Answer 3 · edited May 23 '17 at 10:32

OP's code has trouble with memory allocation and it shows up in file input.

The best is to isolate these 2 functions of code.

Given each line has 3 items: a name, number and amount, let us do that first.

There are so many weaknesses to using fscanf() @William Pursell , going directly to fgets() to read a line of text. Then parse it.

Rather than OP's "%20s%d%f", (which should be "%19s%d%f", code below uses a more complex parse, yet flexible should the name size limit change. Rather than checking the return value of sscanf(), this code checks the last parsed field. Code could check both.

int Read_Record(FILE *istream, char *name, size_t name_size, int *number, double *amount) {
  // No need to be stingy
  #define LINE_SIZE 256
  char line[LINE_SIZE];

  if (fgets(line, sizeof line, istream) == NULL) {
    return EOF;
  }
  // %n saves the offset of scanning at that point.
  int n1;
  int n2;
  int n3 = -1;
  sscanf(line, " %n%*s%n%d%lf %n", &n1, &n2, number, amount, &n3);

  // If scanning did not complete or had extra junk on the line, fail
  if (n3 < 0 || line[n3] != '\0') {
    return 1; // parse failure
  }
  if (n2 - n1 >= name_size) {
    return 2; // name too long
  }
  memcpy(name, line + n1, n2-n1);
  name[n2-n1] = '\0';
  return 0; //success
}

To allocate memory for numOfAccounts records, best to use a structure. As OP is a learner, let us do this without struct.

unsigned numOfAccounts = ...;
// TBD: Add test to insure numOfAccounts > 0

// Notice this style of defining the size needed - simple and clean
char **accountNames = malloc(sizeof *accountNames * numOfAccounts);
int *accountNums = malloc(sizeof *accountNums * numOfAccounts);
double *amounts = malloc(sizeof *amounts * numOfAccounts);
// TBD: Add allocation checks

for (unsigned i = 0; i < numOfAccounts; i++) {
  #define NAME_SIZE 20
  char name[NAME_SIZE];
  int rv = Read_Record(istream, name, sizeof name, &accountNums[i],
      &amounts[i]);
  if (rv != 0) Handle_Error();
  accountNums[i] = strdup(name);
}

// Use data

// free when done.

float is a weak choice for money. double is better. Many issues with any choice.

`strdup()` is not a standard C library function, but common. [Sample code](http://stackoverflow.com/a/39694254/2410359) — chux - Reinstate Monica, Nov 18 '16 at 19:03

fscanf returning value of 0 - C

3 Answers3