Linked Lists: Reading in txt file, nodes dont save

Question

I'm rather new at coding, and was given a tough assignment using linked lists and structures. The assignment is to make a music database such that you can read in artists albums and tracks, and store them in nodes.

Currently, when I run the program, no nodes are being stored when I read in data from a .txt file. Ultimately, I am starting to doubt that my use of certain pointers is correct, as I am getting segmentation faults. What am I doing wrong, and is there a fix I can make that will make it function?

My code (the needed libraries have been included):

Structure and macro definitions:

#define LINEBUFFERSIZE 256

struct song
{
    char *songName_p;
    int trackNumber;
    struct song *nextSong_p;
};

struct disc
{
    char *discName_p;
    int year;
    struct song *song_p;
    struct disc *nextDisc_p;

};

struct artist
{
    char name[20];
    char *artistName_p;
    struct disc *disc_p;
    struct artist *nextArtist_p;
};
struct artist *end = (struct artist *) NULL;   //NEW
struct artist *startPtr = (struct artist *) NULL; //NEW
struct artist *find(struct artist *, char * );//NEW
//NEW

Typedefs:

typedef struct artist artist_t;
typedef struct disc disc_t;
typedef struct song song_t;

typedef struct artist *artistNodePtr;
typedef struct disc *discNodePtr;
typedef struct song *songNodePtr;

Function Definitions:

void InsertArtist(struct artist *New);
artistNodePtr initializenode(char *name);
artistNodePtr findOrInsertArtist(artistNodePtr *sPtr, char *name);
discNodePtr  findOrInsertDisc(discNodePtr *sPtr, char *discID, int releaseYear);
void  findOrInsertSong(songNodePtr *sPtr, char *songID, int trackID);
void getNextLine(char buffer[], int bufferSize, FILE *fptr);
void printlist( struct artist *ptr );
void printnode(struct artist *ptr);

Main Method:

int main(int argc, char *argv[])
{
    char name[20];
    struct artist *newNodePointer;
    char lineBuffer[LINEBUFFERSIZE];
    artistNodePtr startPtr = NULL; /* initially the artist list is empty */
    FILE *musicFile;
    char *artistTemp, *discTemp, *yearTemp, *trackTemp, *songTemp;
    int year, track, menu = 1;
    artistNodePtr theArtist;
    // discNodePtr theDisc;

    if (argc==1)
    {
        printf(" Must supply a file name as command line argument/n");
        return 0;
    }

    if ((musicFile = fopen(argv[1], "r")) == NULL)
    {
        printf ("Error opening music file.  Program terminated/n");
        return 0;
    }

    getNextLine(lineBuffer, LINEBUFFERSIZE, musicFile);
    while (!feof(musicFile))
    {
        artistTemp = strtok(lineBuffer,";");
        if (artistTemp == NULL)
        {
            printf("Error parsing input file; Program is terminated\n");
            return 0;
        }
        discTemp = strtok(NULL ,";");
        if (discTemp == NULL)
        {
            printf("Error parsing input file; Program is terminated\n");
            return 0;
        }
        yearTemp  = strtok(NULL ,";");
        if (yearTemp == NULL)
        {
            printf("Error parsing input file; Program is terminated\n");
            return 0;
        }
        trackTemp = strtok(NULL ,";");
        if (trackTemp == NULL)
        {
            printf("Error parsing input file; Program is terminated\n");
            return 0;
         }
        songTemp = strtok(NULL ,"\n");
        if (songTemp == NULL)
        {
            printf("Error parsing input file; Program is terminated\n");
            return 0;
        }
        year = atoi(yearTemp);
        track = atoi(trackTemp);
        theArtist = findOrInsertArtist(&startPtr, artistTemp);
  //    theDisc = findOrInsertDisc(&(theArtist->disc_p), discTemp, year);
        //findOrInsertSong(&(theDisc->song_p), songTemp, track);
        getNextLine(lineBuffer, LINEBUFFERSIZE, musicFile);
    }  /* end of while loop */

    while (menu != 0)
    {
        printf("1 to display entire catalog \n");
        printf("2 to display alls songs by a given artist\n");
        printf("3 to display all songs on a given disc\n");
        printf("0 to exit the library\n ");
        scanf("%d", &menu);
        switch (menu){
        case 1: printlist(startPtr);
            break;

        case 2:
            printf("enter an artist name");
            scanf("%s", name );
            newNodePointer = find(startPtr, name );
            if (newNodePointer==NULL)
            {
                newNodePointer = initializenode(name );
                InsertArtist(newNodePointer);
            }

        }
    } /* end main */
}

Function definitions:

artistNodePtr findOrInsertArtist(artistNodePtr *sPtr, char *name )
   {

sPtr = initializenode(name);
    InsertArtist(sPtr);

    if(!sPtr)
        sPtr = find(startPtr, name);
    return sPtr;
}
void printlist( struct artist *ptr ){
    while(ptr!= NULL){
    printnode(ptr);
    ptr = ptr->nextArtist_p;
    }
    }
void printnode(struct artist *ptr)
{
    printf("Name %s\n", ptr->name);

}

artistNodePtr initializenode(char *name)
{

    struct artist *newNodePtr = (artistNodePtr)malloc(sizeof(artist_t));
    if (newNodePtr != NULL)
    {
        newNodePtr->artistName_p = (char*)malloc((strlen(name)+1)*sizeof(char));
        if (newNodePtr->artistName_p != NULL)
        {
           strcpy(newNodePtr->artistName_p, name);
            newNodePtr->nextArtist_p = NULL;
            newNodePtr->disc_p = NULL;

            return newNodePtr;
        }   
    }
}

void InsertArtist(struct artist *New)
   {
    //NEW
    struct artist *temp, *prev;

    if(startPtr == NULL)
    {
        startPtr = New;
        end = New;
       startPtr->nextArtist_p = NULL;
        return;
    }
    temp = startPtr;

    while(strcmp(temp->name, New->name) < 0)
    {
        temp = temp->nextArtist_p;
        if(temp == NULL)
            break;
     }
    if(temp == startPtr)
    {
        New->nextArtist_p = startPtr;
        startPtr = New;
    }
    else
    {
       prev = startPtr;
        while(prev->nextArtist_p != temp)
        {
            prev = prev->nextArtist_p;
        }
        prev->nextArtist_p = New;
        New-> nextArtist_p = temp;
            if(end == prev)
            end = New;
    }
}

 struct artist *find(struct artist *newNodePointer, char *name)
{
    //NEW
    while (strcmp(name, newNodePointer->name )!=0)
    {
        newNodePointer = newNodePointer->nextArtist_p;
        if (newNodePointer == NULL)
            break;
    }
    return newNodePointer;
}


void getNextLine(char buffer[], int bufferSize, FILE *fptr)
{
    char temp;
    int i = 0;

    buffer[0] = fgetc(fptr);
    while ( (!feof(fptr)) && (buffer[i] != '\n') &&  i<(bufferSize-1))
    {
        i = i +1;
        buffer[i]=fgetc(fptr);
    }

    if ((i == (bufferSize-1)) && (buffer[i] != '\n'))
    {
        temp = fgetc(fptr);
        while (temp != '\n')
        {
            temp = fgetc(fptr);
        }
     }

    buffer[i] = '\0';
}    

Answer 1

As a start compile with, if you are using GCC,

gcc -Wall -Wextra -pedantic -o myprog myprog.c

On my system I get:

song.c: In function ‘main’:
song.c:75:16: warning: variable ‘theArtist’ set but not used [-Wunused-but-set-variable]
song.c:74:12: warning: variable ‘track’ set but not used [-Wunused-but-set-variable]
song.c:74:6 : warning: variable ‘year’ set but not used [-Wunused-but-set-variable]

song.c: In function ‘findOrInsertArtist’:
song.c:162:7: warning: assignment from incompatible pointer type [enabled by default]
song.c:163:2: warning: passing argument 1 of ‘InsertArtist’ from incompatible pointer type [enabled by default]
song.c:50:6 : note: expected ‘struct artist *’ but argument is of type ‘struct artist **’
song.c:166:8: warning: assignment from incompatible pointer type [enabled by default]
song.c:167:2: warning: return from incompatible pointer type [enabled by default]

song.c: In function ‘initializenode’:
song.c:197:1: warning: control reaches end of non-void function [-Wreturn-type]

song.c: In function ‘main’:
song.c:158:1: warning: control reaches end of non-void function [-Wreturn-type]

Take special heed to everything but -Wunused, then check if those unused variables should have been used and there is some fail to the logic of the code.

Then, when you manage to compile without warnings run the program with valgrind (if on windows I'm not sure; perhaps you can find something useful here: is-there-a-good-valgrind-substitute-for-windows)

---

Edit:
I notice you say you use Code::Blocks. I would recommend using an editor like Vim etc. + compiling on command line. Especially to begin with.

Even so; as mentioned in comment above, you can modify warning level by going to:

"Settings" > "Compiler and debugger ..." > [Compiler Flags]=>[Warnings]

To get debug add -ggdb to [Other options]. If you only want to change this on a project base - you find the same options under:

"Project" > "Build options ..."

---

When you have compiled with debug symbols you can run the program in debug mode.

Simple procedure:

1. Right-click somewhere in your code where you want the execution to stop. Select "Toggle Breakpoint".

2. F8 starts the program in debug mode.

3. Then press F7 to step line by line.

4. ... (Basic keys are in "Debug" menu.)

While running, you can also right-click a variable and select Watch, i.e. "watch 'discTemp'". And, if not present, select "Debug" > "Debugging windows" > "Watches".

Now you will see the value etc. of the variable as you step trough the code.

gdb can also be used on command line. $ gdb -args ./my_prog arg arg

Valgrind, as mentioned before, is also a very useful tool. You will get information about bad behavior in your process. Even if the code compile without error or warning, it is not said that the program is sane.

Valgrind is also implemented in Code::Blocks, even better would perhaps be to say:

$ valgrind ./my_prog arg arg ...

And as a last point: compile all the time. Write a few lines. Compile. Make one change compile. ...

---

These are some hints on how to make the coding much less painful. Hope it helps.

---

Beside the notes mentioned by other people here I also want to add:

• In your menu loop, you have to check if scanf() returns 1, (1 being successfully read elements, "%d" in your case) - and if not, empty buffer. As it is now anything but an integer would result in an infinite loop.

• In getNextLine() you have a while() loop at the end that say get character from file while character is not newline. If file does not end in newline this is another infinite loop. Also read the comments by @sarnold and @torek regarding this function.

---

Ps: To add command line arguments in Code::Blocks you have to use:

"Project" > "Set program's arguments"

Answer 2

There are a bunch of type errors that will spill out with -Wall (as suggested by @user120115).

A few things that jump out right away:

• don't cast malloc (in C; don't even use malloc in C++ unless there's some good reason to avoid new)
• any loop of the form while (!feof(stream)) is probably wrong, because feof does not predict a future EOF, it tells you only why a previous read-attempt failed (e.g., why getchar returned EOF). The point of feof is to distinguish between "normal" read failure due to end of file, and "unusual" (ferror) read failure due to Disk Drive Caught Fire or whatever.
• InsertArtist needs to modify a list, so it either has to return the new list, or take a pointer to the old pointer-to-list, but it does neither. (But the various findOrInsert functions do!)

I'm also with @sarnold on limiting the use of typedef, although this is admittedly a matter of taste.