adding to linked list from external file

cghv · #1 09-Mar-2005, 12:11

Hi,

I am writing a program at the moment which requires me to scan in data from an external file, which to be added to a linked list. The external file is a list of pairs of cities and the distances between them - each piece of data is tab-delimited, and each pair of cities is on a new line. Each node within the linked list should contain the name of city one, the name of city two, and the distance between them. My code so far is shown below:

CPP / C++ / C Code:

typedef struct node_pair_info // declare structure for npi database
{
char city1[20];
char city2[20];
int distance;
struct node_pair_info *next;
} NPI;



NPI * add_nodes(NPI *item){
int i;
FILE *cities;

cities = fopen("ukcities.txt", "r");
if (cities == NULL)
printf("Unable to open ukcities.txt\n");

else 
{

NPI *new_node;
new_node = (NPI *) malloc(sizeof(NPI));
new_node->next = NULL;
fscanf(cities, "%s \t %s \t %d\n", new_node->city1, new_node->city2, new_node->distance);
gotoxy(20,25);
fprintf(stdout, "%s %s %d\n", new_node->city1, new_node->city2, new_node->distance);

}

fclose(cities);

return 0;
}



int main(void)
{
NPI *first=NULL, *current=NULL, *last=NULL;
char response;

system("cls");

gotoxy(25,18); printf("Enter 'a' to add all cars to list");
gotoxy(40,25); putch(' ');
gotoxy(25,25);
printf("Enter command: ");
response = getch();
gotoxy(34,40); putch(response);

switch (response)
{
case 'a': current = last = add_nodes(last); break;
}

if (first == NULL) current = first = last;

return 0;

}

The way which i intend for it to work at the moment is that upon the user entering 'a', all the data will be added to the linked list. The program compiles, but upon running, crashes. Im not certain that the code is correct! If anybody could lend me a hand i would be most grateful!

Thanks,

Charlie

LuciWiz · #2 09-Mar-2005, 13:13

Hello Charlie.
At a quick look, the problem is simple:

Quote:

Originally Posted by cghv

CPP / C++ / C Code:

NPI *new_node;
new_node = (NPI *) malloc(sizeof(NPI));
new_node->next = NULL;

I see you reserving memory, but I don't see you giving it back. Every malloc should be matched by a free when you are done with it.

Now, that is the problem. Try finding a solution (freeing the memory). If you don't succeed, please post back.

Kind regards,
Lucian

nkhambal · #3 09-Mar-2005, 13:34

Hi Charlie,

Couple of problems that I see,

In add_node () function ,by definition, you are supposed to return the pointer to the node of type NPI, instead you are returning 0.

argument data type of function add_node should actually be of type add_node(NPI **item) as you are actually passing a pointer to the pointer. Ideally in your program you are not using this passed argument at all. Instead you are creating a new node and mallocing to it. So argument to function add_node can be blank. However, the return value should be the pointer to new node cause thats what you are assigning to current and last in main().

If you correct these problems, you should be fine. Ofcourse as "LuciWiz" said, every malloc should be paired with free() in your program.

davekw7x · #4 09-Mar-2005, 13:36

Quote:

Originally Posted by cghv

The program compiles, but upon running, crashes. Im not certain that the code is correct! If anybody could lend me a hand i would be most grateful!

Thanks,

Charlie

Let's see: Code is correct, but program crashes.

Well, how could that happen? Off of the top of my head, I can think of only 2,584 reasons

1. Compiler created bad executable from good code.

2. Computer hardware error causes this program to crash.

3. Cosmic rays corrupted memory contents when I tried to run the program.

4.
..
..

(To save time, skip to the bottom of the list: the last resort)

2,584. There is, alas, a Program Bug after all.

Let's start at the bottom and work our way up.

In the first place, the way to debug a program (after you have checked the code enough times that you are sick of looking at it) is to put output statements throughout the code to see where the problem lies. So: get rid of all of the clrscr() and gotoxy() stuff for now, so that you can actually see what's happening.

In the second place you don't ever check return values from malloc() or fscanf(). I think it's a good idea to do this every time. Now, malloc() is not likely to fail to allocate memory (unless, of course there is some program bug), but check it anyhow. On the other hand fscanf() or any function that gets input (from console or file) should always(always) be checked to see that the proper number of input items was obtained. It's not really hard to make a program that works correctly with good input (well, some are harder than others), but you should always (did I say always) check program input data.

I have redone some of your code to show what I have in mind:

CPP / C++ / C Code:

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

typedef struct node_pair_info {
              char city1[20];
              char city2[20];
              int distance;
              struct node_pair_info *next;
            } NPI;

int main(void)
{
  NPI * add_nodes(NPI *item);

  NPI *first   = NULL;
  NPI *current = NULL;
  NPI *last    = NULL;

  char response;

  printf("Enter 'a' to add all cars to list: ");
  response = getchar();


  switch (response) {
    case 'a': printf("Calling add_nodes\n");
              current = last = add_nodes(last); 
              break;

    default: printf("Invalid command\n");
  }
  printf("returned from add_nodes, with value = %p\n", last);
  if (last) {
    free(last);
  }
  return 0;

}

NPI * add_nodes(NPI *item)
{
  int i;
  FILE *cities;
  char temparray1[BUFSIZ] /* new stuff here for debugging */;
  char temparray2[BUFSIZ];
  int tempint;
  int number_of_items_scanned;
  NPI *new_node; /* note that we must return this value so that it can
                    be free()ed later
                  */

  cities = fopen("ukcities.txt", "r");
  if (cities == NULL) {
    printf("Unable to open ukcities.txt\n");
    return NULL;
  }
  else {

    printf("Opened input file ukcities.txt\n");

    new_node = (NPI *) malloc(sizeof(NPI));
    if (!new_node) {
      printf("Error: can't allocate %d bytes\n", sizeof(NPI));
      return NULL;
    }
    else {
      printf("Allocated %d bytes for new_node.\n", sizeof(NPI));
    }
    new_node->next = NULL;
    number_of_items_scanned = fscanf(cities, "%s \t %s \t %d\n",
            temparray1, temparray2, &tempint);
    printf("Number of items on this line = %d\n", number_of_items_scanned);
    printf("temparray1: <%s>\n", temparray1);
    printf("temparray2: <%s>\n", temparray2);
    printf("tempint   =  %d\n", tempint);
    strcpy(new_node->city1, temparray1);
    strcpy(new_node->city2, temparray2);
    new_node->distance = tempint;
    printf("new_node->city1:    <%s>\n", new_node->city1);
    printf("new_node->city2:    <%s>\n", new_node->city2);
    printf("new_node->distance = %d\n", new_node->distance);

#if 0
    fscanf(cities, "%s \t %s \t %d\n", 
        new_node->city1, new_node->city2, new_node->distance);
    fprintf(stdout, "%s %s %d\n", 
            new_node->city1, new_node->city2, new_node->distance);
#endif
  }

  fclose(cities);

  return new_node; 
}

Now, try this and see if it compiles and executes to your expectation. Note a few minor fixes that I installed in addition to the debug information.

(Note that I commented around your fscanf() for test purposes.)

If this works ok but your fscanf() causes it to crash, maybe you can look a little closer at your fscanf(). Hint: There is a bug here.

Regards,

Dave

p.s. I didn't mean my reply to be sarcastic towards you or anybody else on Spaceship Earth. This is my own mental process for approaching a program that doesn't perform as expected: Try, try, try to find someone or something else to blame. Then get to work.

Keep the Faith,
D

Thread Tools	Search this Thread
Show Printable Version Email this Page	Search this Thread: Advanced Search
Rate This Thread
Rate This Thread:

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
Airport Log program using 3D linked List : problem reading from file	batrsau	C Programming Language	11	29-Feb-2008 07:44
[Include] Doubly-linked List	dsmith	C Programming Language	6	14-Apr-2006 13:12
CD burner wont burn!!	robertli55	Computer Hardware Forum	1	18-Jun-2004 10:53
Yet another CD burner problem: Lite-On LSC-24082K	Erwin	Computer Hardware Forum	1	22-May-2004 11:28