0

this is an assignment for my CS course, im trying to write a code that reads a file line by line and put the input into a struct element.the struct looks like this:

typedef char* Name;
struct Room
{
    int     fStatus;
    Name    fGuest;
};

the status is 0 for available and 1 for booked. the name will be empty if the room is available.

there are 2 function, one to read and put the values to a struct element, and the other one to print it out.

int openRoomFile()
{
    FILE *roomFile;
    char *buffer = NULL;
    size_t length = 0;
    size_t count = 0;

    roomFile = fopen("roomstatus.txt", "r+");
    if (roomFile == NULL)
        return 1;

    while (getline(&buffer, &length, roomFile) != -1) {
        if (count % 2 == 0) {
            sscanf(buffer, "%d", &AllRooms[count].fStatus);
        } else {
            AllRooms[count].fGuest = buffer;
        }
        count++;
    }

    fclose(roomFile);
    free(buffer);
    return 0;
}

print function

void printLayout(const struct Room rooms[])
{
    for (int i=0; i<3; i++) {
        printf("%3d \t", rooms[i].fStatus);
        puts(rooms[i].fGuest);
    }
}

the output is not what i expected, given the input file is :

1
Johnson
0

1
Emilda

i will get the output :

1   (null)
0   
0   (null)

i dont know what went wrong, am i using the right way to read the file? every code is adapted from different sources on the internet.

justomat
  • 67
  • 1
  • 6

3 Answers3

1

Here is a fixed version of the openRoomFile()

int openRoomFile(void)
{
    FILE *roomFile;
    char *buffer = NULL;
    size_t length = 0;
    size_t count = 0;

    roomFile = fopen("roomstatus.txt", "r+");
    if (roomFile == NULL)
        return 1;

    while (1) {
        buffer = NULL;
        if (getline(&buffer, &length, roomFile) == -1) {
            break;
        }
        sscanf(buffer, "%d", &AllRooms[count].fStatus);
        free(buffer);

        buffer = NULL;
        if (getline(&buffer, &length, roomFile) == -1) {
            fprintf(stderr, "syntax error\n");
            return 1;
        }
        AllRooms[count].fGuest = buffer;
        count++;
    }

    fclose(roomFile);
    return 0;
}

When you no longer need those fGuest anymore, you should call free on them.

Lee Duhem
  • 14,695
  • 3
  • 29
  • 47
  • You're leaking like a sieve! You need to insert `free(buffer);` before the second `buffer = NULL;` in the loop. (The first is still needed, so that the first guest's name does not get overwritten with other data.) – Jonathan Leffler Apr 01 '14 at 05:46
  • @leeduhem Hi! thanks for the answer. Just one quick question, why do i need to free the buffer and then set it to null (3 lines just after the `sscanf`). Doesnt `free` disallocate memory? why the buffer is still usable after the `free` call? btw it compiles and runs as what i wanted. – justomat Apr 01 '14 at 17:31
  • @user3060941 If the dereference of its first argument is `NULL`, `getline()` will `malloc` enough memory to store the input and store the address of that buffer to the pointer pointed by its first argument. When you do not need that buffer anymore, you should free it to prevent memory leak. – Lee Duhem Apr 02 '14 at 01:06
0

If your input is guaranteed to be valid (as were many of my inputs in my CS classes), I'd use something like this for reading in the file.

while(!feof(ifp)){
    fscanf(ifp,"%d%s",&AllRooms[i].fStatus, AllRooms[i].fGuest); //syntax might not be right here
                                                                //might need to play with the '&'s
                                                                //and maybe make the dots into
                                                                //arrows

     //do work here
    i++;
}
slinhart
  • 562
  • 5
  • 17
  • Oh, you are using `feof()` wrongly, see http://stackoverflow.com/questions/5431941/while-feof-file-is-always-wrong – Lee Duhem Apr 01 '14 at 04:16
  • Ya your right, I remember reading about that awhile ago. You can still get around it though if you are guranteed that your file isnt empty to start with. Then instead of having the !feof(ifp) in the while loop, Just add a check for if(!feof(ifp) then break; at the end of the loop. I know it's not the most elegant code ever, but it gets the job done. – slinhart Apr 01 '14 at 04:33
  • You also are not taking care of the memory allocation for the `Name` type, which is a `char *` in disguise. You might use the POSIX [`sscanf()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/sscanf.html) functionality `%ms`, or you might use [`strdup()`](http://pubs.opengroup.org/onlinepubs/9699919799/functions/strdup.html) or you might roll your own string duplicator. But you must ensure memory is allocated somewhere. – Jonathan Leffler Apr 01 '14 at 05:48
0

You are not allocating memory for Name. Check this. In the below example i'm not included free() calls to allocated memory. you need to call free from each pointer in AllRooms array, once you feel you are done with those and no more required.

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

typedef char* Name;
struct Room
{
    int     fStatus;
    Name    fGuest;
}Room_t;
struct Room AllRooms[10];

int openRoomFile()
{
    FILE *roomFile;
    char *buffer = NULL;
    size_t length = 0;
    size_t count = 0;
    size_t itemCount = 0;

    roomFile = fopen("roomstatus.txt", "r+");
    if (roomFile == NULL)
        return 1;
    buffer = (char *) malloc(16); // considering name size as 16 bytes
    while (getline(&buffer, &length, roomFile) != -1) {
        if (count % 2 == 0) {
            sscanf(buffer, "%d", &AllRooms[itemCount].fStatus);
        } else {
            AllRooms[itemCount].fGuest = buffer;
        itemCount++;
        }
        count++;
    buffer = (char *) malloc(16); // considering name size as 16 bytes
    }

    fclose(roomFile);
    free(buffer);
    return 0;
}
void printLayout(const struct Room rooms[])
{
    int i;
    for (i=0; i<3; i++) {
        printf("%3d \t", rooms[i].fStatus);
        puts(rooms[i].fGuest);
    }
}

int main(void)
{
  openRoomFile();
  printLayout(AllRooms);
  // free all memory allocated using malloc()
  return 0;
}
user207064
  • 665
  • 5
  • 14