Loading files into lists

This is a discussion on Loading files into lists within the C Programming forums, part of the General Programming Boards category; I've been working on this for a few days now, and I'm seriously stuck on this function. For this part, ...

  1. #1
    Registered User
    Join Date
    Sep 2008
    Location
    Orlando, Florida
    Posts
    22

    Loading files into lists

    I've been working on this for a few days now, and I'm seriously stuck on this function. For this part, I'm supposed to read in a file and create a linked list from the data in the file. I have my program reading in the files, no problem. What I am running into is when it tries to insert the data into the list, the program just crashes. Can someone maybe explain what I am doing wrong?

    Here is the code to load the file (it is in my main):

    Code:
    else if(choice == 5)
            {
              printf("Enter load file name.\n");
              scanf("%s", &name2); 
              
              fload = fopen(name2, "r");
              
              while(!feof(fload))
              {
                    fscanf(fload, "%s %s %d %s %s", make, model, &year, color, license);
                    printf("%s %s %d %s %s \n", make, model, year, color, license);  //To make sure the file is being read properly                
                    if(insert(&head, make, model, year, color, license))
                    {
                        printf("file loaded\n");
                    }
              }
              
              printf("File loaded!\n");
              
              fclose(fload);
            }


    And here is my insert function (which will work with the rest of the program):

    Code:
    int insert(struct vehicle** head, char* make, char* model, int year, char* color, char* license)
    {
        struct vehicle* newvehicle;
        struct vehicle* crnt = (*head);
        
        newvehicle = malloc(sizeof(struct vehicle));
        newvehicle->make = (char*)calloc(strlen(make)+1, sizeof(char));
        newvehicle->model = (char*)calloc(strlen(model)+1, sizeof(char));
        newvehicle->color = (char*)calloc(strlen(color)+1, sizeof(char));
        newvehicle->license = (char*)calloc(strlen(license)+1, sizeof(char));
        
        strcpy(newvehicle->make,make);
        strcpy(newvehicle->model,model);
        newvehicle->year = year;
        strcpy(newvehicle->color,color);
        strcpy(newvehicle->license,license);
        
        
        
        if ((*head) == NULL || (strcmp((*head)->license , license) > 0))
        {
            newvehicle->next = (*head);
            (*head) = newvehicle;
            return 1;
        }
        
        
        while(crnt->next != NULL && (strcmp(crnt->next->license , license) < 0))
        {
            crnt = crnt->next;
        }
        
        if(strcmp(crnt->next->license, license) == 0)
            return 0;
        
        
        newvehicle->next = crnt->next;
        crnt->next = newvehicle;
        return 1;
    }

  2. #2
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,458
    1. scanf("&#37;s", &name2);
    & should be removed

    2. while(!feof(fload))
    do not use feof for control loops - read FAQ

    3. you do not check if the head->license is equivalent to the new license

    4. if the new license is already present - you have a memory leak - no need to allocate memory

    5. no need to cast calloc
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  3. #3
    Registered User
    Join Date
    Sep 2008
    Location
    Orlando, Florida
    Posts
    22
    Alright, I changed the !feof and the &name, but now it is not reading anything from the file at all. I used this line:

    Code:
    while(fscanf(fload, "%s %s %d %s %s", make, model, &year, color, license) == 1)
    Should I have used something different?

  4. #4
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    fscanf() returns the number of elements successfully read.
    Code:
    fscanf(fload, "%s %s %d %s %s", make, model, &year, color, license)
    attempts to read 5 elements. The only time that function will return 1 is if ONLY make could be read (which usually would mean that you have a borked input file).

    And whilst this is a good excercise in memory allocation, I would think that having some reasonable fixed length fields for make, model, colour, and license would make more sense. Dynamically allocated memory takes up some extra bytes, quite often 16 or 32 bytes on top of the actual memory you allocated plus it is rounded up to the nearest multiple of 8 or more.

    So, to make an example, allocating memory for a short name like "Ford" will take up 5 bytes for the name, 3 bytes as it's rounded to 8 bytes, 4 bytes for the pointer, 16 bytes of overhead. That is 28 bytes. If we assume that most car names are less than 20 characters, then we SAVE 8 bytes by having a fixed array of 20 bytes [I don't know of any car manufacturer that has a longer name (although there may be several I haven't heard of), unless you write out BMW as "Bayerische Motoren Werke AG" or something equally silly.]

    The models are likely to be about the same, whilst colour and license are shorter (unless you want the colour to be Green And Pink With Red Spots).

    It is also a waste of time to call calloc when you are doing a strcpy immediately after, since strcpy will fill every byte of the memory you have allocated in itself. [Of course, if you make the change I just suggested, it would make sense to use calloc to allocate the vehicle struct itself].

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  5. #5
    Registered User
    Join Date
    Sep 2008
    Location
    Orlando, Florida
    Posts
    22
    Alright, I moved the code down into a function, and added some room up at the top such as:

    Code:
    char license[20];
    char make[20]; 
    char model[20];
    char color[20];
    This allows me to read the file, but as soon as I try to send it through the insert function, it just crashes again.

  6. #6
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Code:
        while(crnt->next != NULL && (strcmp(crnt->next->license , license) < 0))
        {
            crnt = crnt->next;
        }
        
        if(strcmp(crnt->next->license, license) == 0)
            return 0;
    If crnt->next is NULL, you will try to get license from a NULL-pointer when the loop ends.

    Otherwise, add some code to indicate where you are going through the insert function, and print out as many pointers as you can. If that doesn't help, you probably have to post the code again, as it must have changed.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  7. #7
    Registered User
    Join Date
    Sep 2008
    Posts
    21
    There seems to be a lot of questions about linked lists on this board. This is how I approach them.

    First, in order to simplify the explanation I am going to use a very simple structure.
    It will only contain the key or search value and the reference to the next list item.

    struct list_item {
    char key[20];
    struct list_item *next;
    };

    This structure could be expanded to any number of members of various types,
    but keep it simple for now. After the list functions correctly you can then add
    them.

    I am going to assume that the key values in this list are to be unique. That's
    kind of a trick statement, because without that constraint the list would be
    ambigious. So ...

    Code:
    struct list_item *head;
    char token[128];
    
    head = NULL; /* initialize empty list */
    
    if(f3 = fopen("source.txt","r")) != NULL){
      while(fscanf(f3,"%s",token) != EOF){
    
    /* You don't have to pass in a pointer to head. head is already a pointer 
    and as such the L value will be passed to the function */
    
        switch(finsert(head,token)){
          case 0:
             printf("%s inserted\n",token);
             break;
          case 1:
             printf("%s already in list\n",token);
             break;
          default:
             printf("What ????\n");
             break;
        }
    
      }
      fclose(f3):
    }
    
    finsert (head,token)
    
    struct list_item *head;
    char *token;
    
    {
    
      struct list_item *current; /* pointer to the current place in the list */
    
      /* if haed is equal to NULL (empty list) add element */
    
      if(head == NULL){
    
        head = ((struct list_item *) (calloc(1, sizeof(struct list_item))));
        strcpy(head->key,token);
        head->next = NULL;
        return(0);
    
      } else {
    
        current = head; /* initialize current to the head of the list */
    
    /* the loop to check if the value is already there. If it is return 1 */
    
        while(current != NULL){
          if(strcmp(token,current->key) == 0){
            return(1);
          }
    
    /* if current->next equals NULL you are at the end of the list and should add the element otherwise continue to walk the list */
    
          if(current->next == NULL){
             current->next = ((struct list_item *) (calloc(1, sizeof(struct list_item))));
             strcpy(current->next->key,token);
             current->next->next = NULL;
             return(0);
          } else {
            current = current->next;
          }
        }
      }
    
    }

  8. #8
    and the Hat of Guessing tabstop's Avatar
    Join Date
    Nov 2007
    Posts
    14,185
    Does this work for you? Those pre-C89 compilers must have been something else, being able to modify the value of something passed in (like head in finsert).

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. added start menu crashes game
    By avgprogamerjoe in forum Game Programming
    Replies: 6
    Last Post: 08-29-2007, 01:30 PM
  2. Replies: 4
    Last Post: 12-11-2004, 01:36 PM
  3. need help w/ linked lists
    By MKashlev in forum C++ Programming
    Replies: 11
    Last Post: 08-05-2002, 08:57 PM
  4. Loading large files with c++
    By Unregistered in forum Linux Programming
    Replies: 1
    Last Post: 03-17-2002, 11:26 PM
  5. Loading all files in a directory
    By steve_i83 in forum Windows Programming
    Replies: 2
    Last Post: 08-26-2001, 02:18 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21