C - Change the position of the conditions in while loop leads to Segmentation fault in C?

Question

I have a function to insert an element to a hash table like this in my program:

void insert(int key,int data)
{
    struct DataItem *newdata = (struct DataItem*) malloc(sizeof(struct DataItem));
    int  hashIndex;

    newdata->key = key;
    newdata->data = data;

    hashIndex = hashCode(key);

    while(hashArray[hashIndex] != NULL && hashArray[hashIndex]->key != -1)
    {
        ++hashIndex;
        hashIndex %= MAX_SIZE;
    }   

    hashArray[hashIndex] = newdata;
}

This works without any problem. However, when I change the position of the condition in while loop to this:

while(hashArray[hashIndex]->key != -1 && hashArray[hashIndex] != NULL )

There ís an "Segmentation Fault" error. And the program pointer stops at the while loop when I call the insert function.

I'm wondering what is the problem?

[Don't cast the result of `malloc` in C](http://stackoverflow.com/q/605845/995714) — phuclv, Aug 16 '17 at 04:23

score 2 · Accepted Answer · edited Aug 16 '17 at 04:42

2

(hashArray[hashIndex] != NULL && hashArray[hashIndex]->key != -1)

It works because here, if first condition is false, the second one does not get executed — that is behavior of && operator — so in case hashArray[hashIndex] is NULL, it does not access invalid memory by accessing hashArray[hashIndex]->key

edited Aug 16 '17 at 04:42

Jonathan Leffler

730,956
141
904
1,278

answered Aug 16 '17 at 04:22

Pras

4,047
10
20

score 1 · Answer 2 · answered Aug 16 '17 at 04:23

C has short circuit evaluation for boolean &&. This means that if the first expression evaluates to false, the second won't be evaluated and false will be returned directly.

Now in your case

while(hashArray[hashIndex] != NULL && hashArray[hashIndex]->key != -1)

if hashArray[hashIndex] turns out to be NULL, the second part won't be evaluated.

This means you are safe.

But what happens when you switch the order and hashArray[hashIndex] is indeed NULL? You dereference NULL before checking it and hence you get a Seg Fault.

So the best solution for you is to keep it as is.

score 1 · Answer 3 · edited Aug 16 '17 at 04:43

The problem is that in the first case you first check hashArray[hashIndex] != NULL and access key member only if the object exists. The second version will attempt to access the key member before making sure that object was created.

In other words, in the second version it may happen that hashArray[hashIndex] is NULL and so your first part of the condition (hashArray[hashIndex]->key) is equivalent to NULL->key, which leads to access violation.

C - Change the position of the conditions in while loop leads to Segmentation fault in C?

3 Answers3