Buffer overflow or something else

Question

I am creating a program, about seat reservations. I was asked to use unsigned short and unsigned int for some of the variables, so that is why they are set like that. I have a program that works ok. But when I transfer everything inside a function, everything seems to work ok, but inside my structure weird values start to be saved all over the place.. I only want to save the values of the file (from line 2 -> the end of file). Because I have a structure that to be initialized I have first to read the txt file and numberofseats, I have am declaring this variable (passenger) 2 times..inside the function (local var) and in the main body.. Maybe this causes the problem? If I don't use a function everything work fine!

So the problematic code:

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

int i,j,numberofseats,temp;
char platenr[8],selection,buff[60];
char firstname[20];
char lastname[20];
char phone[11];
char *p;
typedef struct
    {
    char fullname[40];
    unsigned short phonenr[10];
    unsigned int seatnr;
    }PASSENGERS;

void readfile( void)
{
    FILE *businfo;
    businfo = fopen ("bus.txt","r");
    if (businfo == NULL)
        {
        printf("Error Opening File, check if file bus.txt is present");
        exit(1);}
    else
        {
        fscanf(businfo,"%s %d",platenr, &numberofseats);
        printf("Bus Licence plate Nr is: %s and number of seats is: %d", platenr, numberofseats);
        PASSENGERS passenger[numberofseats];
        for (j=0;j<numberofseats;j++)
            {passenger[j].seatnr=j+1;
            strcpy(passenger[j].fullname,"\0");
            }
        while (fgets(buff,sizeof(buff),businfo)!=0)
            {sscanf(buff, "%s %s %d %s", firstname, lastname, &temp,phone);
            strcpy(passenger[temp-1].fullname,firstname);
            strcat (passenger[temp-1].fullname, " ");
            strcat(passenger[temp-1].fullname,lastname);
            printf("%s",passenger[temp-1].fullname);
            i=0;
            for (p=phone;*p!='\0';p++)
                {
                (passenger[temp-1].phonenr[i])=*p -'0';
                i++;
                }
            }
           }
}

int main(void)
{
readfile();
PASSENGERS passenger[numberofseats];

Answer 1

Ok, so what I did, before starting to work on dynamic allocation is specify the max number of seats in the start of main, and from there I finished my code as follows. I have 2 warning messages though in lines 43, 109 that can't seem to be able to fix.

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

int i,j,numberofseats,temp;
char platenr[8],selection;
char firstname[20],lastname[20];
char phone[11];
char *p;
typedef struct
    {
    char fullname[40];
    unsigned short phonenr[10];
    unsigned int seatnr;
    }PASSENGERS;

void readfile(PASSENGERS passenger[])
{   char buff[60];
    FILE *businfo;
    businfo = fopen ("bus.txt","r");
    if (businfo == NULL)
        {
        printf("Error Opening File, check if file bus.txt is present");
        exit(1);}
    else
        {
        fscanf(businfo,"%s %d",platenr, &numberofseats);
        printf("Bus Licence plate Nr is: %s, and Number of Seats is: %d.", platenr, numberofseats);

        for (j=0;j<numberofseats;j++)
            {passenger[j].seatnr=j+1;
            strcpy(passenger[j].fullname,"\0");
            }
        while (fgets(buff,sizeof(buff),businfo)!=0)
            {sscanf(buff, "%s %s %d %s", firstname, lastname, &temp,phone);
            strcpy(passenger[temp-1].fullname,firstname);
            strcat (passenger[temp-1].fullname, " ");
            strcat(passenger[temp-1].fullname,lastname);
            i=0;
            for (p=phone;*p!='\0';p++)
                {
                (passenger[temp-1].phonenr[i])=*p -'0';
                i++;
                }
            }
           }
}

void countfreeseats(PASSENGERS passenger[]){
    int freeseats = 0;
        for (j=0; j<numberofseats; j++)
            {
            strcmp(passenger[j].fullname,"\0")==0 ? freeseats = freeseats + 1 : freeseats ;}
            printf ("There are %d Free Seats in this Bus. \n", freeseats);
            printf("Seats that are Available are:\n");

        for (j=0; j<numberofseats; j++)
            {if (strcmp(passenger[j].fullname,"\0")==0)
                printf ("%u\n", passenger[j].seatnr);
            }
        freeseats = 0;
        }

void changeData(PASSENGERS *target){
    unsigned short tempdigit;
    printf("Enter Passenger's first name:");
    scanf("%s",firstname);
    printf("Enter Passenger's last name:");
    scanf("%s",lastname);
    strcpy(target->fullname,firstname);
    strcat (target->fullname, " ");
    strcat(target->fullname,lastname);
    printf("Enter Passenger's phone Nr:");
    scanf("%s",phone);
    i=0;
    for (p=phone;*p!='\0';p++)
        {
        (target->phonenr[i])=*p -'0';
        i++;
        }


    }

void searchpassenger(PASSENGERS passenger[], char selection)
{       char tempsel,tmpfirst[20],tmplast[20];
        unsigned short tempphone[10];
     if (selection == '1')
        {   printf("Enter Passenger's first name:");
            scanf("%s",tmpfirst);
            printf("Enter Passenger's last name:");
            scanf("%s",tmplast);
            strcat (tmpfirst, " ");
            strcat(tmpfirst,tmplast);
            for (j=0;j<numberofseats;j++)
            if (strcmp(passenger[j].fullname,tmpfirst)==0)
                printf ("Passenger %s has Seat Nr #: %u\n",tmpfirst,passenger[j].seatnr);
        }
        else if (selection == '2')
        {   printf("Enter Passenger's Phone Nr:");
            scanf("%s",phone);
            i=0;
            for (p=phone;*p!='\0';p++)
            {
            (tempphone[i])=*p -'0';
            i++;
            }
            for (j=0;j<numberofseats;j++)
            {if (strcmp(tempphone,passenger[j].phonenr)==0)
                printf("Passenger %s has Seat Nr %hd already Booked",passenger[j].fullname,passenger[j].seatnr);
            }
        }
        }



void cancelSeat(PASSENGERS *target){
    strcpy(target->fullname,"\0");
    for (i=0;i<10;i++)
    target->phonenr[i]=0;
    printf("Seat Nr %d is now Free",temp);

    }

void showList(PASSENGERS passenger[])
{
 printf("The following Seats are Booked: \n Name, PhoneNr, SeatNr\n\n");                                    /*Emfanisi minimatos*/
        for (i=0; i<numberofseats; i++)
            if (strcmp(passenger[i].fullname,"\0")!=0)
            {
                printf("%s, ",passenger[i].fullname);
                for (j=0;j<10;j++)
                {printf("%hu",passenger[i].phonenr[j]);}

                printf(", %u\n",passenger[i].seatnr);
            }
}


void writeFile(PASSENGERS passenger[])
   {
    FILE * output;                                      /* Dilosi onomatos arxeiou */
    output = fopen("output.txt","w");    /*dimiourgia i eggrafi pano se iparxon arxeio me onoma output.txt,  mesw tis parametrou w*/
    fprintf(output,"%s %d \n",platenr,numberofseats);    /* mesw tis fprintf eksagogi pinakidas kai epikefalidas "Diagramma leoforeiou" sto arxeio output.txt. Allagi grammis opou xreiazetai*/
    for (i=0; i<numberofseats; i++)
        {if (strcmp(passenger[i].fullname,"\0")!=0)
        {
            fprintf(output,"%s ",passenger[i].fullname);
            fprintf(output,"%u ",passenger[i].seatnr);
            for (j=0;j<10;j++)
            fprintf(output,"%hu",passenger[i].phonenr[j]);
            fprintf(output,"%s","\n");
        }
        }
        fclose(output);                                     /* Kleisimo arxeiou*/
        printf("File Saved as Output.txt");
   }

int main(void)
{


PASSENGERS passenger[53];
readfile(passenger);

    do{
    printf("\n\nNeo Sistima Katagrafis Thesewn Leoforeiou\n");
    printf("Please make a selection:\n\n");
    printf("0. Exit\n");
    printf("1. Empty Seats \n");
    printf("2. Book Specific Seat \n");
    printf("3. Advanced Search of Booked Seats\n");
    printf("4. Cancel Seat Booking\n");
    printf("5. Show List of Booked Seats\n");
    scanf(" %c",&selection);

    if (selection=='1')
  countfreeseats(passenger);

    else if (selection=='2')
        {
        printf("Please give seat nr (between 1 and %d) that you want to book:\n", numberofseats);
        scanf("%d",&temp);
        if (temp >numberofseats || temp <= 0)
            {printf("Error: Seat nr should be between 1 and %d", numberofseats);}
         else if (strcmp(passenger[temp-1].fullname,"\0")!=0)
            printf("Error: Seat is already booked");

        else
        changeData(&passenger[temp-1]);

        }

 else if (selection=='3')
        {
        char tempsel;
        printf("Do you want to search with Name (1) or Phone Nr (2)?\n");
        scanf(" %c",&tempsel);
        searchpassenger(passenger,tempsel);
        }





    else if (selection=='4')
        {
        printf("Please give Seat Nr (between 1 and %d) that you want to Cancel Booking:\n", numberofseats);
        scanf("%d",&temp);
        if (temp >numberofseats || temp <= 0)
            {printf("Error: Seat nr should be between 1 and %d", numberofseats);}
         else if (strcmp(passenger[temp-1].fullname,"\0")==0)
            printf("Error: Seat is already free");

        else
        cancelSeat(&passenger[temp-1]);

        }

    else if (selection=='5')                                                            /*Menu 6 - Emfanisi listas kratimenon thesewn taksinomimenon kata ayksonta arithmo*/
        {
            showList(passenger);

        }



    } while (selection!='0');
    {
    writeFile(passenger);
    }


    }

Answer 2

The problem is that you are declaring a local array in the function readfile(), and once this function terminates, it is lost. You need to be able to return the changes to main(). For that you have some options. One is that you may declare the array in main(), and change your function to void readfile(PASSENGERS passenger[]). In this case, you will do something like this:

int main()
{
PASSENGERS passenger[numberofseats];
readfile(passenger);
// more code

You will be basically passing a pointer to the memory location of the elements stored in the array, local to main(), and the function will fill the array, effectively returning the changes.

Another option is to dynamically allocate an array (with malloc() family) in the function, and make it return a pointer like PASSENGERS *readfile(void). This option may be more suitable if the number of seats is not known at compile time, so you need to dynamically grow or shrink the array when necessary. This option however, leaves you the burden of managing the memory manually, like free()'ing the allocated memory when you are done.

Since you say that you will read numberofseats from the file, the latter would be the better idea, so your code will look something like this:

PASSENGERS *readfile(void)
{ 
   FILE *businfo;
   PASSENGERS *passenger;
   businfo = fopen ("bus.txt","r");
   // do the checks, read the numberofseats
   passenger = malloc(numberofseats * sizeof *passenger);
   // read the values, fill the array
   fclose(businfo); // do not forget to close the file
   return passenger;
}

int main()
{
    PASSENGERS *passenger = readfile();
    // more code
    free(passenger);
    return 0;
}

Answer 3

A variable called x in function foo has nothing to do with a variable called y in function bar. In other words: passenger in main and passenger in readfile are different variables. Changing one will not impact the other.

What you want is probably more like this:

int main(void)
{
    PASSENGERS passenger[numberofseats];
    readfile(passenger);
             ^^^^^^^^^
             Pass array as a pointer
    ....
}

and

void readfile(PASSENGERS* passenger)
{
    ....

    // REMOVE THIS: PASSENGERS passenger[numberofseats];


}

Beside that notice:

// Global variables gets zero initialized
int i,j,numberofseats,temp;
        ^^^^^^^^^^^^
        Becomes zero at start up

but still you use it in main:

PASSENGERS passenger[numberofseats];

That is probably no what you really want.

Since you try to read the number of seats in the function, it seams you really want to use dynamic memory allocation. Like:

PASSENGERS* readfile()
{
     .....
     .....

     PASSENGERS* p = malloc(numberofseats * sizeof(PASSENGERS));

     .....
     .....
     return p;
}

int main(void)
{
    PASSENGERS* passenger = readfile();
     .....
     .....
     free(passenger);
     return 0;
}

If you don't want dynamic allocation, you must move the input of numberofseats into main so it is done before declaring the array.