0

I have made a program for hotel management. It has a problem that: In the Accounts function, it should reset all the flags of the registered members through the program. But there is a error that it does NOT. I have spent a lot of months, trying to debug this problem, but i couldn't. So please help me. Here is the code of the account function,

void accounts()
{
    int ttt=0;
    struct person payment;
    char aname[21], oname[21];
    char *namea;
    int chec=1, ver=0;
    long int recsize;
    recsize=sizeof(payment);
    f=fopen("C:\\HOTEL.DAT", "rb+");
    if(f == NULL){
        clrscr();
        cprintf("File could not be opened!");
        sleep(4);
        exit(0);
    }
    clrscr();
    cprintf("\n                   *** Pearl Guest House - Payments ***\n\n");
    cprintf("\r\r\rEnter the Name :");
    fflush(stdin);
    scanf("%[^\n]s", &aname);
    namea=strupr(aname);
    strcpy(oname, namea);
    while(fread(&payment, recsize, 1, f) == 1){

        if((payment.flag == 1) && (strcmp(payment.name,oname) == 0)){
            payment.pay=1;
            printf("\n\n Payment Received");
            fflush(stdin);
            getch();
            ver=1;

            fseek(f, -recsize, SEEK_CUR);
            fwrite(&payment, sizeof(payment), 1, f);
            break;

        }

    }

    if(ver!=1){
        printf("\n\n Record not Found!!!");
        fflush(stdin);
        getch();
    }
    //rewind(f);
    fclose(f);
    f=fopen("C:\\HOTEL.DAT", "rb+");
    if(f == NULL){
        clrscr();
        cprintf("File could not be opened!");
        sleep(4);
        exit(0);
    }

    while(fread(&payment, recsize, 1, f) == 1){
        if(payment.pay==0){
            chec=0;
            break;
        }
    }
    //rewind(f);
    f=fopen("C:\\HOTEL.DAT", "rb+");
    if(f == NULL){
        clrscr();
        cprintf("File could not be opened!");
        sleep(4);
        exit(0);
    }

    if(chec==1){
            while(fread(&payment, recsize, 1, f) == 1){
                payment.pay=0;
                fseek(f, -recsize, SEEK_CUR);
                fwrite(&payment, recsize, 1, f);
                ttt++;
                printf("%d", ttt);
            }

            printf("\n\n All payments recieved...\n\nSo, the payments flags are set to 0");
            fflush(stdin);
            getch();
    }
    printf("Before Fclose");
    fclose(f);
    printf("After Fclose");
}

This is my structure person: 

struct s_office{   
    char name[16];
    char phone[12];
    };
struct permanent{
    char addr[100];
    char phone[12];
    };

struct emergency{
    char name[21];
    char relation[11];
    char phone[12];
};

struct person{
    char name[21];
    char phone[12];
    char place[21];
    int roomno;
    int flag;
    char food;
    struct s_office office;
    char father[21];
    char fphone[12];
    struct permanent per;
    struct emergency emer1;
    char email[40];
    int finger;
    char dob[8];
    int cidate;
    int cimonth;
    int ciyear;
    int codate;
    int comonth;
    int coyear;
    int rent;
    int pay;
};

Here is a list of headers i included:

#include<conio.h>
#include<stdio.h>
#include<stdlib.h>
#include<ctype.h>
#include<string.h>
#include<dos.h>

I used Turbo C++ to write this code. When this is run, it runs successfully but the output is not what is supposed to be. (It is supposed to open the file(where is store data - HOTEL.DAT), read all the written functions one by one and then if it sees that all of them have pay variable = 1; it should say "All payments received..." and then it should set all of them to 0) I tried to debug and found out that most probably, the error is here:

while(fread(&payment, recsize, 1, f) == 1){
                payment.pay=0;
                fseek(f, -recsize, SEEK_CUR);
                fwrite(&payment, recsize, 1, f);
                ttt++;
                printf("%d", ttt);
            }

Here I suppose that the loop is ok because if i comment the contents of the loop, it runs n times. (if n = number of entries) BUT if i uncomment the contents it does not! This is the problem. Therefore it doesn't set all of them to 0. What i want is a solution to this problem and code so that it gives desired output(of setting every pay variable to 0)

Remember: This is a 14 year old kid trying to code; please help and answer politely and in detail;;; THANKS

I have moved my code to Visual Studio Express 2012 and now it shows ONE MORE PROBLEM It goes into an infinity loop in accounts and creates thousands entries!!!! Here is the code

void accounts()
{
    int ttt=0;
    struct person payment;
    char aname[21], oname[21];
    char *namea;
    int chec=1, ver=0;
    long int recsize;
    recsize=sizeof(payment);
    f=fopen("D:\\HOTEL.DAT", "rb+");
    if(f == NULL){
        system("cls");
        cprintf("File could not be opened!");
        MySleep(4);
        exit(0);
    }
    system("cls");
    cprintf("\n                   *** Pearl Guest House - Payments ***\n\n");
    cprintf("\r\r\rEnter the Name :");
    fflush(stdin);
    scanf("%[^\n]s", &aname);
    namea=strupr(aname);
    strcpy(oname, namea);
    while(fread(&payment, recsize, 1, f) == 1){

        if((payment.flag == 1) && (strcmp(payment.name,oname) == 0)){
            payment.pay=1;
            printf("\n\n Payment Received");
            fflush(stdin);
            getch();
            ver=1;

            fseek(f, -recsize, SEEK_CUR);
            fwrite(&payment, sizeof(payment), 1, f);
            break;

        }

    }

    if(ver!=1){
        printf("\n\n Record not Found!!!");
        fflush(stdin);
        getch();
    }
    //rewind(f);
    fclose(f);
    f=fopen("D:\\HOTEL.DAT", "rb+");
    if(f == NULL){
        system("cls");
        cprintf("File could not be opened!");
        MySleep(4);
        exit(0);
    }

    while(fread(&payment, recsize, 1, f) == 1){
        if(payment.pay==0){
            chec=0;
            break;
        }
    }
    //rewind(f);
    f=fopen("D:\\HOTEL.DAT", "rb+");
    if(f == NULL){
        system("cls");
        cprintf("File could not be opened!");
        MySleep(4);
        exit(0);
    }

    if(chec==1){
            while(fread(&payment, recsize, 1, f) == 1){
                payment.pay=0;
                fseek(f, -recsize, SEEK_CUR);
                fwrite(&payment, recsize, 1, f);
                ttt++;
                printf("%d", ttt);
            }

            printf("\n\n All payments recieved...\n\nSo, the payments flags are set to 0");
            fflush(stdin);
            getch();
    }
    printf("Before Fclose");
    fclose(f);
    printf("After Fclose");
}
  • 2
    Why on earth would you use Turbo C++? The newest version is 7 years old... – codeling Oct 01 '13 at 10:37
  • 4
    I don't believe people still write such code and apply to it C++ tag. – SChepurin Oct 01 '13 at 10:38
  • 2
    (1) this is c code, not c++. (2) try to find a **short** and self-contained example showing the problem. (3) write what output you get and what you expect (4) show what you did try to resolve the problem yourself – codeling Oct 01 '13 at 10:39
  • 1
    @nyarlathotep someone may be using turbo c++ because one may only have access to really old computers that can only run dos and newer compilers may not generate code for it – Ahmed Masud Oct 01 '13 at 10:43
  • `fflush(stdin)` results in undefined behaviour on most platforms. – Paul R Oct 01 '13 at 10:43
  • @AhmedMasud: if a computer is new enough to run a web browser and access StackOverflow then it is new enough to run a reasonably up-to-date compiler. – Paul R Oct 01 '13 at 10:45
  • 2
    In "a lot of months", you could as well have rewritten the whole program, this time using unit tests to check that each part works correctly. – Daniel Daranas Oct 01 '13 at 10:45
  • 2
    @PaulR Why do you assume that he's developing this for his OWN computer? I know I have access to a super computer and still write code that would use repurposed cash-machine register CPUs because that's all my client has available in the country they live in. – Ahmed Masud Oct 01 '13 at 10:46
  • 2
    Yes that is one reason for why am i using Turbo C++ but anyways i am moving on to a newer compiler. BUT that doesnt really matter; because if the code is right, it should work no matter which compiler i am using. I have used Turbo C++ Compiler for quite some time and i think that there is definitely no problem in it due to which this code is giving error –  Oct 01 '13 at 10:46
  • 1
    @nyarlathotep Sir i have tried a lot and it is on a thread on some other website, if you want i can give you a link. People over there couldn't help. I hope getting some help here –  Oct 01 '13 at 10:47
  • 1
    Sir i have tried to rewrite the whole code, it didn't help –  Oct 01 '13 at 10:48
  • If the error is in the loop you think it is, I would recommend putting in more debug output code in there (i.e. more `printf` statement). Also, check the result of fseek and fwrite! – codeling Oct 01 '13 at 11:17
  • 1
    @nyarlathotep Sir, i have done that; as i told you fread is right as; if i comment it works fine. then there are only two statements left in the loop fseek and fwrite. I have used this combination in other functions in the program and over there it is working fine. So i dont think that there should be any problem. And i have uploaded whole of the code of accounts function so that you can see if the problem is not where i am saying –  Oct 01 '13 at 11:21
  • You are using `sizeof struct` in your seek routines. That makes struct padding a prime suspect. – Jongware Oct 01 '13 at 11:27
  • 1
    @Jongware What is structure padding? and one more thing; i tried to replace it with sizeof directly but no change. What do you suggest??? –  Oct 01 '13 at 11:29
  • Google, or read [c-structure padding and packing](http://stackoverflow.com/questions/4306186/structure-padding-and-structure-packing) to find out what it is. Then (1) disable it (if your compiler allows it) or (2) read and write structure variables one at a time. But first, *confirm* this is the problem! Print `sizeof payment`, manually add up the sizes of each member, and check if they match or not. – Jongware Oct 01 '13 at 11:36
  • 1
    @Jongware Sir i will surely see that, but the problem shouldn't be this i suppose because according to this problem i wont be able to run the code like this in any way. I mean that if i comment the code inside the loop, the loop works fine so that loop problem should not be there. What do you say? –  Oct 01 '13 at 11:40
  • 1
    @Jongaware And sir i am using Turbo C++ so it doesn't have that option. i am trying to update the compiler and the time i do so successfully i will try it. But for the time been, can u please run it and check? –  Oct 01 '13 at 11:42
  • What is `ttt` in your loop? – RedX Oct 01 '13 at 11:46
  • You should put a `printf` and print the name of the person next to the line `check=0;` – Klas Lindbäck Oct 01 '13 at 11:57
  • 1
    @RedX ttt is just a variable used for debugging nothing else –  Oct 01 '13 at 12:13
  • 1
    @KlasLindbäck Sir i did that but no much achievement from that. It doesn't help. If commented, it shows name of all people and if uncommented it shows name of some people. But the logic i am using is fine according to me. If u don't believe try it yourself i gave u the Code! –  Oct 01 '13 at 12:15
  • 3
    Suggest 1. Check results of `fseek()` and `fwrite()`. 2. Perform `fflush(f)` after the `fwrite()` and check `fflsuh()` result. – chux - Reinstate Monica Oct 01 '13 at 12:22
  • 1
    @chux how to check fflush() result??? –  Oct 01 '13 at 12:39
  • @Daksh Shah fflsuh retun value: "A zero value indicates success. If an error occurs, EOF is returned and the error indicator is set (see ferror)." `if (fflush(f) != 0) handle_error`. – chux - Reinstate Monica Oct 01 '13 at 12:41
  • @chux: *Theoretically* this should not matter: "For files open for update (those which include a "+" sign), [..] the stream should be flushed (fflush) **or repositioned** (fseek, fsetpos, rewind) between either a writing operation followed by a reading operation or a reading operation which did not reach the end-of-file followed by a writing operation." But worth a try. – Jongware Oct 01 '13 at 12:53
  • 1
    Ah, wait. The above quote mentions *either*. Now you have 'read, seek, write', and next a 'read' again. So a `fflush` after `fwrite` ought to solve it, as @chux notes. – Jongware Oct 01 '13 at 12:59
  • If you are a 14 years old kid, consider installing Linux on your PC (assuming your PC has e.g. at least a gigabyte of RAM, and a dozen gigabytes of disk, i.e. has less than 5 years). You'll learn a lot. Then use (under Linux) [GCC](http://gcc.gnu.org/) as `gcc -Wall -g` and debug with `gdb` – Basile Starynkevitch May 22 '14 at 11:46

1 Answers1

2

Have you run this code in a debugger to see if you can determine what is wrong? Without doing that, we're all taking stabs in the dark...

Anyway, you have 2 possible problems that I can see.

  • This piece of code will not work consistently:
    
    while(fread(&payment, recsize, 1, f) == 1){
                payment.pay=0;
                fseek(f, -recsize, SEEK_CUR);
                fwrite(&payment, recsize, 1, f);
                ttt++;
                printf("%d", ttt);
            }

At chux stated in the comments, you should call fflush(f) after fwrite. fwrite and fflush both can and will return errors and you're not checking for the errors or handling them. This could also be the source of your corruption.

  • Structure packing (as stated by Jongware in the comments)

Structure packing means a couple of things:

  • The amount of padding put between fields in a structure
  • The amount of padding added to the end of a structure to cause it to end on a natural boundary. Most 32 bit machines like their data to start on a 32 bit boundary.

Some machines will not work with unaligned fields and require some hackery, meaning, for instance, that a 32 bit int should start on a 32bit aligned boundary and end on a 32bit aligned boundary. I can think of at least one processor that used page faults to deal with this situation which slowed the code down immensely. Page faults are an order of magnitude slower than direct memory reads and writes, so structure packing becomes very important.

On Intel x86 processors, the penalty is not very high for misaligned reads/writes so it is somewhat safe to turn off structure packing. Look at the Turbo C++ documentation for #pragma pack. This will lead you to the reading you need to do to understand the issue. Once you understand the compiler documentation, go search the internet and read about structure padding/packing.

Make sure you only have one definition of each structure.

I feel like I'm forgetting something, but.... Hopefully this will get you started.

JimR
  • 15,513
  • 2
  • 20
  • 26