0

I'm writing a simple banking application in c It saves the information in a file. I want to load the file each time the app runs, and add the information from the file to the struct, for this purpose, I have written two functions called "loadfile" and "allocate"

In the function "loadfile" if I un-comment the comment lines, the OS throws a "stopped working" in my face :|

Can you help me with it ? when I use (acc+i) in the "loadfile", the error shows up. Is there a syntax problem? :o thanks

typedef struct {
    char name[20];
    int id;
    int balance;
    char branch[10];
} account;

account *acc;

int allocate ( account *acc )  {
    int num = 0 ;
    char tempname[20],tempbranch[10];
    int tempid = -1 ,tempbalance;
    FILE *file;
    file = fopen("D://bank.txt","r");
    while ( !feof(file) ) {
        fscanf(file,"%s %d %d %s ",tempname, &tempid, &tempbalance, tempbranch);
        if (tempid != -1)
            num++;
    }
    acc = ( account *) realloc ( acc, num * sizeof(account) );
    fclose(file);
    printf(" num in allocate function : %d",num);
    return num;
}

int loadfile (account *acc) {
    int num = allocate(acc);
    char tempname[20],tempbranch[10];
    int tempid ,tempbalance;
    if ( num != 0 ) {
        int i = 0 ;
        FILE *file;
        file = fopen("D:\\bank.txt","r+");
        for ( i = 0 ; !feof(file) && i < num ; i++ ) {
            fscanf(file,"%s ",tempname );
            fscanf(file,"%d ",&tempid );
            fscanf(file,"%d ",&tempbalance );
            fscanf(file,"%s ",tempbranch );
            printf("\n i is %d \n",i);
            /* strcpy( ((acc+i)->name) , tempname);
            (acc+i)->id = tempid;
            (acc+i)->balance = tempbalance;
            strcpy( ((acc+i)->branch) , tempbranch); */
        }
        fclose(file);
    }
    return num;
}
SOFUser
  • 89
  • 6
  • 2
    [why-is-while-feof-file-always-wrong](http://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) – LPs Dec 23 '16 at 07:40
  • 2
    Use Global `account *acc;` I.E `int allocate ( account *acc ) {` --> `int allocate (void) {` – BLUEPIXY Dec 23 '16 at 07:42
  • Don't cast the return of `void *` function! – Stargateur Dec 23 '16 at 07:45
  • 1
    You have too many mistake in this code. It don't verify return value and it don't use correctly `feof()`. Plus you don't know what is the scope in C and your format style is bad. – Stargateur Dec 23 '16 at 07:56
  • @SOFUser There are 2 ways you can use realloc: Either you use it to change the size of a segment previously allocated. You must then pass a pointer to previously allocated memory. Or you use it like a malloc to allocate new memory. You must then pass a NULL pointer. You do the latter seemingly just out of luck, because you are using an icky global variable with static storage duration, which is as it happens initialized to NULL. Should you move that variable to a local scope, where it should be, then realloc won't work unless you explicitly initialize the pointer to NULL. – Lundin Dec 23 '16 at 08:46
  • I don't see much sense in the `allocate` function in the first place. Just use [an expansion `realloc` algorithm](http://pastebin.com/iGaex0KZ) in your `loadfile`. Scanning the file *twice* seems overkill. – WhozCraig Dec 23 '16 at 08:54
  • @WhozCraig And optimize the capacity of realloc seems overkill too. And down vote without say why is bad. I take time to write this answer. – Stargateur Dec 23 '16 at 09:06

2 Answers2

1

There are a lot of issues with the posted code. It is unclear how a separate allocation function is helpful, and the use of the file-scope variable acc is not advisable. I see no point in using realloc() here, since allocation is done only once. If you do use realloc(), you should store the result in a temporary pointer, because the function can return a NULL pointer if there is an allocation error. This leads to a memory leak if you assign directly to the pointer that you are reallocating from. I have rewritten the code to illustrate some fixes, trying to maintain the general structure of the original code.

You should check the return values of the functions that you call. realloc() and malloc() return a pointer to the allocated memory, or a NULL pointer in the event of an allocation error. You should check this value and handle the result. The scanf() functions return the number of successful assignments made. You should check this value to verify that input is as expected. It is almost always a bad idea to control a loop with feof(), because this function relies upon the end-of-file indicator being set, and this indicator is only set when an I/O operation has failed.

In the program below, fgets() is used to read a line of input from the file into a buffer, and sscanf() is used to extract the input data from the buffer. In the allocation phase, EOF or an empty line signals the end of the data. No parsing is done here, only a count of lines. For each line, space is allocated for an account. Note that the code checks for errors on opening and closing the file, as well as for an allocation error.

The loadfile() function again uses fgets() to read a line of input into buffer, and then uses sscanf() to scan the buffer. Note the use of width specifiers for the strings. These are one less than the size of the arrays that they read into, to leave space for the '\0' placed at the end of the strings by sscanf(). Also note that in the event that fewer than 4 assignments are made, the program exits with an error message. If the assignments to the temporary variables were successful, the account is updated with the data.

There are many ways that this code could be improved (most obviously by getting rid of the global variable acc), but this should provide you with a good starting point.

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

typedef struct {
    char name[20];
    int id;
    int balance;
    char branch[10];
} account;

account *acc = NULL;

int allocate(void)
{
    int num = 0 ;
    char buffer[1000];
    FILE *file;

    file = fopen("D://bank.txt","r");
    if (file == NULL) {
        fprintf(stderr, "Unable to open file in allocate()\n");
        exit(EXIT_FAILURE);
    }

    while (fgets(buffer, sizeof(buffer), file) != NULL &&
           buffer[0] != '\n') {
        num++;
    }

    acc = malloc(num * sizeof(*acc));
    if (acc == NULL) {
        fprintf(stderr, "Allocation error in allocate()\n");
        exit(EXIT_FAILURE);
    }

    if (fclose(file) != 0) {
        fprintf(stderr, "Unable to close file in allocate()\n");
        exit(EXIT_FAILURE);
    }

    printf(" num in allocate function : %d\n",num);
    return num;
}

int loadfile(void)
{
    int num = allocate();
    char buffer[1000], tempname[20],tempbranch[10];
    int tempid ,tempbalance;
    if ( num != 0 ) {
        int i = 0 ;
        FILE *file;

        file = fopen("D://bank.txt","r+");
        if (file == NULL) {
            fprintf(stderr, "Unable to open file in loadfile()\n");
            exit(EXIT_FAILURE);
        }

        while (fgets(buffer, sizeof(buffer), file) != NULL
               && buffer[0] != '\n') {
            if (sscanf(buffer, "%19s %d %d %9s",
                       tempname, &tempid, &tempbalance, tempbranch) != 4) {
                fprintf(stderr, "%d: Malformed input data\n", i);
                exit(EXIT_FAILURE);
            }

            strcpy(acc[i].name, tempname);
            acc[i].id = tempid;
            acc[i].balance = tempbalance;
            strcpy(acc[i].branch, tempbranch);
            ++i;
        }

        if (fclose(file) != 0) {
            fprintf(stderr, "Unable to open file in loadfile()\n");
            exit(EXIT_FAILURE);
        }
    }

    return num;
}

int main(void)
{
    int num = loadfile();
    for (int i = 0; i < num; i++) {
        printf("%s %d %d %s\n",
               acc[i].name, acc[i].id, acc[i].balance, acc[i].branch);
    }

    return 0;
}
Community
  • 1
  • 1
ad absurdum
  • 19,498
  • 5
  • 37
  • 60
-1

I can't explain all your problems. It will take me hours. I hope this code will be self-describing. Ask me in a comment if you need some help.

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

typedef struct {
  char name[20];
  int id;
  int balance;
  char branch[10];
} account_t;

static account_t *parse_account_file(FILE *file, size_t *size) {
  if (file == NULL || size == NULL) {
    return NULL;
  }

  size_t i = 0;
  account_t *account = malloc(sizeof *account);
  if (account == NULL) {
    return NULL;
  }

  int ret;
  while (
      (ret = fscanf(file, "%19s %d %d %9s\n", account[i].name, &account[i].id,
                    &account[i].balance, account[i].branch)) == 4) {
    account_t *old = account;
    account = realloc(account, sizeof *account * (++i + 1));
    if (account == NULL) {
      free(old);
      return NULL;
    }
  }

  if (ret == EOF) {
    if (ferror(file)) {
      perror("parse_account_file()");
    } else {
      *size = i;
      account_t *old = account;
      account = realloc(account, sizeof *account * i);
      if (account == NULL) {
        return old;
      }
      return account;
    }
  } else {
    fprintf(stderr, "error parsing\n");
  }

  free(account);

  return NULL;
}

int main(void) {
  char const *name = "D:\\bank.txt";
  FILE *file = stdin;

  size_t size;
  account_t *account = parse_account_file(file, &size);
  fclose(file);
  if (account == NULL) {
    return 1;
  }

  for (size_t i = 0; i < size; i++) {
    printf("%s %d %d %s\n", account[i].name, account[i].id, account[i].balance,
           account[i].branch);
  }

  free(account);
}
Stargateur
  • 24,473
  • 8
  • 65
  • 91
  • Not the downvoter, but what is the scope of `return realloc(account, sizeof *account * i);` ??? – LPs Dec 23 '16 at 09:26
  • Side note: using `realloc` reflecting its return value to the original pointer can leak memory previously allocated if `realloc` fails. – LPs Dec 23 '16 at 09:33
  • @LPs I though that `realloc` free if it fail to locate the memory, i will fix that. I never use `realloc` in my program. I return `realloc` (this one can't fail, no?), to tell that I don't use the exceed memory that I allocate http://stackoverflow.com/questions/7078019/using-realloc-to-shrink-the-allocated-memory. – Stargateur Dec 23 '16 at 09:38