Getting a Segmentation Fault 11 Not Sure if Using Pointers Correctly in C

Question

Please keep in mind I am new to C and the whole pointers/memory allocation is a bit tricky to me. Also so is the command line argument input through the terminal.

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

struct node {
    long long key;
    long long val;
    struct node *next;
}; 
struct hashTable {
    struct node *head;
    int buckets;
}; 

int count = 0;

struct hashTable *newTable;

//create a new node
struct node * createNode (long long key, long long val) {

    struct node *newNode;
    newNode = (struct node *) malloc(sizeof(struct node));

    newNode -> key = key;
    newNode -> val = val;
    newNode -> next = NULL;

    return newNode;
}

//insert data into Hash
void insertToHash(long long key, long long val) {
    // hash func
    int hashIndex = key % 1000, inTable = 0;
    struct node *newNode = createNode(key, val);

    //traversal nodes
    struct node *temp, *curr;
    curr = newTable[hashIndex].head;

    //if the table at given index is empty 
    if (newTable[hashIndex].head == NULL) {
        newTable[hashIndex].head = newNode;
        count ++;
        return;
    }

    temp = curr;
    //if key is found break, else traverse till end
    while(curr != NULL) {
        if (curr -> key == key) {
            inTable = 1;
            free(newNode); //since already in the able free its memory
            break;
        }
        else {
            temp = curr;
            curr = curr->next;
        }
    }

    if (inTable == 1) {
        printf("Address is already in the table");
    }
    //key not found so make newNode the head
    else {
        newNode -> next = newTable[hashIndex].head;
        newTable[hashIndex].head = newNode;
        count ++;
    }

}

//initialize hashtable to 1000 entries
struct hashTable * createHashTable (int buckets) {

    int i;
    for(i=0; i<buckets; i++) {
        newTable[i].head = NULL;
    }

    return newTable;
}



int main(int argc, char *argv[]) {

    createHashTable(1000);




}

So when I searched up what a Segmentation Fault 11 was I found out that it has to do with not having access to certain memory. I'm assuming my issue has something to do with initializing the table newTable and not using pointers properly or allocating memory for it properly. Please keep in mind this is my first real attempt to create a data structure in C so things that might seem obvious aren't obvious to me.

`struct hashTable *newTable;` : `newTable` is `NULL`. Can't use like `newTable[i].head = NULL;` — BLUEPIXY, Oct 21 '16 at 18:03
Regarding your secondary but irrelevant question, please see [What are the arguments to main() for?](http://stackoverflow.com/questions/3734111/what-are-the-arguments-to-main-for) — Weather Vane, Oct 21 '16 at 18:03
first thing, you're not allocating any space for `newTable`. You'll need to `malloc` some space for `newTable` .. although I'm guessing what you'll really want is 1 `struct hashTable` with 1000 `struct node`s? In that case, I wouldn't make `newTable` a pointer, just have `struct hashTable newTable;`, and then `malloc` the number of `struct node`s you want. — yano, Oct 21 '16 at 18:08
@yano That is correct I just need a HashTable with 1000 buckets and use chaining for collision. However if I do what you're suggesting and malloc the number of struct nodes that I need, how would I for instance input a value to say hashIndex 50 if I'm not setting up the Hashtable as an array? — SA97, Oct 21 '16 at 18:17
You can index a pointer as if it is an array. You have already done that, but no memory was allocated. — Weather Vane, Oct 21 '16 at 18:20
Be careful.. do you want a linked list? Your `struct node* next` pointer suggests you want a linked list. If you do, `newHash` is basically just the head of the linked list. Linked lists are meant to be dynamic data structures.. they evolve and change during runtime. If that's what you're going to end up doing, then DO NOT pretend `newHash` is a pointer to an array. An array is a contiguous block of memory. If you `malloc`, insert, and delete nodes during runtime, there are no gurantees that `newHash.head[50]` will access anything meaningful, much less the 51st node. If, however, you simply — yano, Oct 21 '16 at 18:42
want to create 1000 nodes at program start and you won't delete or create new ones during runtime, then it is ok to just `malloc` 1000 nodes, make `newHash.head` point to the first one, then you can access each one via array indexing (`newHash.head[##]`). If you take this approach, there's really no need for `struct node* next` in `struct node`. — yano, Oct 21 '16 at 18:45
@yano No I mean I want 1000 buckets and each of those buckets are linked lists, so for instance if my hash function spit out 50 then newHash[50] should bring me to the 50th bucket and insert the value to the linked list on bucket 50. — SA97, Oct 21 '16 at 19:05
ah ok, so you want a linked list of `struct node`s at each bucket? — yano, Oct 21 '16 at 19:25

score 0 · Answer 1 · answered Oct 21 '16 at 18:19

0

Your code layout is great and easy to read!

The following is wrong

for(i=0; i<buckets; i++) {
    newTable[i].head = NULL;
}

Based on

struct hashTable *newTable;

newTable is a pointer to a single struct, not a pointer to an array.

As to correct solution, please first follow the hashtable example in K&R book and then feel free to modify to suit your needs.

answered Oct 21 '16 at 18:19

Arun

19,750
10
51
60

As long as there is enough memory allocated it is ok. But the compiler has no idea how many elements you want to index. If you allocate memory for one element, it will let you index the 100th, but the mistake only happens at run time. It is only because `newTable` is a global initialised variable ( to `NULL`!) that the compiler does not issue a warning. – Weather Vane Oct 21 '16 at 18:22
@weatherVane if I wanted to malloc for 1000 buckets, then how would I do that? that is what confuses me. – SA97 Oct 21 '16 at 18:28
With `newTable = malloc(1000 * sizeof *newTable); if(newTable == NULL) { /* handle error */ }`. But as you have hard-coded the `1000` and as you have a global `newTable` pointer I don't know why you don't just `#define ELEMS 1000` then `struct hashTable newTable[ELEMS];` and be done with it. – Weather Vane Oct 21 '16 at 18:34
@WeatherVane This is my third day with C I'm not sure what #define does. could you elaborate. – SA97 Oct 21 '16 at 18:41
Oh! Please [read this](https://www.tutorialspoint.com/cprogramming/c_preprocessors.htm) about the C preprocessor. Essentially it is text substitution. Wherever `ELEMS` is encountered in the code it is replaced with `1000` (textually). This means if you hard code a number, you only need to do it *once* in a place that you can find and modify if required, and the whole program will follow suit. – Weather Vane Oct 21 '16 at 18:47
@WeatherVane thank you so much I think I'm going to use newTable = (struct hashTable *) malloc(buckets * sizeof(struct hashTable)) because I will have to implement some sort of rehash functionality later in my code. – SA97 Oct 21 '16 at 18:55
@WeatherVane also if I were to to malloc, would that remove the need for having that for loop that sets every bucket to null? – SA97 Oct 21 '16 at 18:56
That is the same, but the beginners way. a) don't cast the return value, b) use the size of what is pointed to, rather than duplicating the type. – Weather Vane Oct 21 '16 at 18:56
No, please read the man pages next, for `malloc` and `calloc`. This is becoming too much like a tutorial. – Weather Vane Oct 21 '16 at 18:57

Getting a Segmentation Fault 11 Not Sure if Using Pointers Correctly in C

1 Answers1