CODEWITHSUNDEEP
c
Sindre Smistad
Sindre Smistad

Reputation: 139

Integer comparison fails

I have a linked list of a node structure, and in my function for searching through the list to find a node with a matching id the if statement seems to fail when comparing he passed in id and the node id. The if statement is on line 6 in the function below. Even if both *node_id* and id has the same value it fails.

NODE *node_id_search(int id, NODE *start) {
    NODE *result = NULL, *current = start;

    do {

        if(current->node_id == id) {
            result == current;
        }

        current = current->next;
    } while(current->next != NULL && result == NULL);


    return result;
}

node.h

typedef struct node {
    /*@{*/
    /**
     * The node id used to identify the node. The id is a positive integer.
     */
    int node_id;

    /**
     * The node type.
     */
    char node_type[10];

    /**
     * Pointer to the next node element.
     */
    struct node *next;
    /*@}*/
} NODE;

Upvotes: 1

Views: 386

Answers (3)

Aniket Inge
Aniket Inge

Reputation: 25723

Other than answers mentioned above(which I don't see how they relate to the question), the only problem I see is this piece of code:

    if(current->node_id == id) {
        result == current; //which should be result = current;
    }

Change it to:

if(current->node_id == id){
     result = current;
     return result; // no need to search any further(hence optimized).
}

Other than that,I don't see any problem with your code.

Upvotes: 2

wildplasser
wildplasser

Reputation: 44250

Your code is overly complicated. It can be reduced to:

NODE *node_id_search(int id, NODE *ptr) {

    for( ; ptr; ptr = ptr->next) {
        if(ptr->node_id == id) return ptr;
        }

    return NULL;
}

BTW: the above snippet returns the first matching node in the chain, where the original returned the last.

Also: if the pointer-argument ("start" in the original) were NULL, the original would dereference the NULL pointer and crash (or return nonsense). This version with the for(;;) loop would just return NULL.

Upvotes: 0

Sufian Latif
Sufian Latif

Reputation: 13356

2 points to be noted in the block

if(current->node_id == id) {
    result == current;
}

  1. You're not checking whether current is NULL or not. If any node with node_id equal to id does not exist, eventually you'll reach the end of the list (where next is NULL) and try to evaluate NULL->next and crash. Put a printf() before this block to see what happens.

  2. You've written result == current, which does nothing on result. It's just checking for equality, and result remains the same forever. It should be result = current, which assigns the value of current to result.

Upvotes: 0

Related Questions