Thread: Having trouble Adding a node to end of linked list

I am trying to add a node to the end of a linked list, and the code I wrote does it successfully, but the program ends up crashing. I think there is something wrong with my while loop, but I'm not sure what!

Code:

/* Point -- A struct to represent a point (x,y) in the plane */
typedef struct {
    float x, y;
} Point;


/* PointListNode -- A node of a singly-linked list of points */
typedef struct PointListNode{
    Point *p;
    struct PointListNode *next;
} PointListNode;

void add_point_at_end(PointListNode *pointList, Point* newPoint){
	 PointListNode *newNode = (PointListNode*)malloc(sizeof(PointListNode));
	 newNode->p->x = newPoint->x;
	 newNode->p->y = newPoint->y;
	 newNode->next = NULL;


	 PointListNode *current = pointList;
	 while (current->next != NULL) {
		 current = current->next;
	 }
	 current->next = newNode;


} /* add_point_at_end */

Your code will crash if pointList is initially NULL. If it is NULL, then you need to set the caller's argument to the new node, either by a return value or a double pointer.

BTW, you could say

Code:

    *newNode->p = *newPoint;

And cool coders write their mallocs like this:

Code:

PointListNode *newNode = malloc(sizeof *newNode);

I should add that it's good style to test whether or not malloc succeeded.

No, it looks right to me, to be honest. The loop needs to look for the node with the NULL next pointer and attach a new node there.

I think what you need to fix is the pointList pointer. See, the head pointer in main is different from the pointer that the function will edit, so you technically have to return the head pointer to main(), or use another asterisk. If you start out with just a NULL pointer, and don't change your function's parameters, the pointer appears to stay the same.

Code:

void add_point(PointListNode **pointList, Point *newPoint);

// Now you can do things like this:
Point p = { 0, 0 };
PointListNode *head = NULL;
scanf("%f,%f", &p.x, &p.y);
add_point(&head, &p);
scanf("%f,%f", &p.x, &p.y);
add_point(&head, &p);
...

Unfortunately the code I am writing is part of an assignment, and I am not allowed to change anything outside of add_point or any of its parameters. So I wouldn't be able change PointListNode to ** . Is there any other way I can go about it?

Originally Posted by TehQii

Is there any other way I can go about it?

Not really.
How is add_point_at_end being called?

The instructor uses a function to test my code to see if it worked, and then the test function is called in main. This is the code to test my function:

Code:

void testAddPointAtEnd(){
    char inputBuffer[MAX_LINE];
    inputBuffer[MAX_LINE-1] = '\0';
    char *inputString;
    PointListNode *list = NULL;
    Point* newPoint = NULL;
    float xy[2];
    int count;
    startTest("add_point_at_end");


    //Now allow the user to enter as many points as they want
    printf("Creating point list - Enter as many points as you want, and leave a line blank when you are done.\n");
    count = 0;
    while (1){
        printf("Enter x & y for point %d: ",count+1);
        fflush(stdout);
        if(!(inputString = testPrompt(NULL,inputBuffer,sizeof(inputBuffer))))
            break;
        if (readNFloats(inputString,xy,2) != 2){
            printf("\t Wrong number of values provided - ending list.\n");
            break;
        }
        list = addPointToList(list,allocateNewPoint(xy[0],xy[1]));
        count++;
    }
    if (count == 0){
        printf("\t No data provided - Skipping test.\n\n");
        return;
    }
    list = reversePointList(list);
    printf("The list of points is: ");
    printPointList(list);
    printf("\n");
    printf("Enter x & y for a point to add at the end of the list: ");
    fflush(stdout);
    if(!(inputString = testPrompt("",inputBuffer,sizeof(inputBuffer))))
        return;
    if (readNFloats(inputString,xy,2) != 2){
        printf("\t Wrong number of values provided - skipping test.\n\n");
        return;
    }
    newPoint = allocateNewPoint(xy[0],xy[1]);
    printf("Adding the point (%.2f,%.2f) to the end of the list with add_point_at_end\n",newPoint->x,newPoint->y);
    add_point_at_end(list,newPoint);
    printf("The modified list of points is: ");
    printPointList(list);
    printf("\n");
    freePointList(list);
    fflush(stdout);
} /* testAddPointAtEnd */

For some reason your instructor has made it impossible for you to write the function correctly. All you can do is maybe test if pointList is NULL and simply return in that case since there's nothing you can do. Or maybe even terminate the program and print an error message. Try the latter first and see if pointList is ever NULL. If not, then the error causing the crash is not in the code you posted.

Originally Posted by Salem

Where does p point to in order for you to dereference it?
You need to allocate some memory, or use some memory allocated for you.

*facepalm*
He needs to allocate memory for the point.
I totally missed that.

The function is still broken by design in that it can't add an element to an empty list, but the missing point memory is the cause of the crash here.