1

I'm looking for some help with some C dll programming. I am getting an error in Visual Studio that occurs when I call free from within the dll. The program runs fine in debug mode within the IDE, but when I try to execute it as "Start without debugging", the program crashes. I read that with debugging, the heap is shared, which probably explains why the code runs fine with F5 and not Ctrl-F5. Is this correct??

I've searched around, and I learned that it is dangerous to pass dynamically allocated memory through the dll-exe boundary, however as I am calling malloc and free from within the same dll, this should not be a problem. I have posted the code below. Any help would be greatly appreciated. Please note that this code works on my Linux box, I am simply trying to port it to Windows to garner experience programming in Windows. Thank you

#ifndef HASHTABLE_H
#define HASHTABLE_H

// The following ifdef block is the standard way of creating macros which make exporting 
// from a DLL simpler. All files within this DLL are compiled with the HASHTABLE_EXPORTS
// symbol defined on the command line. This symbol should not be defined on any project
// that uses this DLL. This way any other project whose source files include this file see 
// HASHTABLE_API functions as being imported from a DLL, whereas this DLL sees symbols
// defined with this macro as being exported.
#ifdef HASHTABLE_EXPORTS
#define HASHTABLE_API __declspec(dllexport)
#else
#define HASHTABLE_API __declspec(dllimport)
#endif

#ifdef __cplusplus
extern "C" {
#endif

/**
  * This is a naive implementation
  * of a hashtable
  */

typedef struct node {
    struct node *next;
    char *value;
    char *key;
} Node;

typedef struct hashtable {
    struct node **nodes;
    int num_elements;
    int size;
    int (*hash_function)(const char * const);
} Hashtable_str;

// Construction and destruction
HASHTABLE_API void tbl_construct(Hashtable_str **table);
HASHTABLE_API void tbl_destruct(Hashtable_str *table);

// Operations
HASHTABLE_API int tbl_insert (Hashtable_str *table, const char * const key, const char * const element); // return the key
HASHTABLE_API int tbl_remove(Hashtable_str *table, const char * const key);
HASHTABLE_API char * tbl_find(Hashtable_str *table, const char * const key); // return the element
HASHTABLE_API int size(Hashtable_str *table); // Return the size

// default hash function
int def_hash(const char * const key);

#ifdef __cplusplus
}
#endif

#endif

Here is the implementation code

// hashtable.cpp : Defines the exported functions for the DLL application.
//

#include "stdafx.h"
#include "hashtable.h"
#include <stdlib.h> // for memcpy
#include <stdio.h>
#include <string.h>

#define SIZE 100

int def_hash(const char * const key)
{
    // simply sum the ascii
    // values and take modulo
    // 100
    //int i;
    //int sum;
    //sum = 0;
    //for (i=0; key[i] != '\0'; i++)
    //    sum += key[i];
    //return sum % SIZE;

    return 0;
}

// construct a hashtable and return a pointer
HASHTABLE_API void tbl_construct(Hashtable_str **tbl)
{
    int i;
    Hashtable_str *tbl_ptr;
    *tbl = (Hashtable_str*) malloc (sizeof(Hashtable_str*));
    tbl_ptr = *tbl;

    tbl_ptr->nodes = (Node**) malloc (SIZE * sizeof(Node*));
    for (i=0; i < SIZE; i++) tbl_ptr->nodes[i] = NULL;
    tbl_ptr->hash_function = &def_hash;
    tbl_ptr->num_elements = 0;
    tbl_ptr->size = SIZE;
}

HASHTABLE_API void tbl_destruct(Hashtable_str *tbl)
{
    void free_tbl_node(Node*); // declare the release function

    int i;
    for (i=0; i < tbl->size; i++)
    {
        if (tbl->nodes[i] != NULL)
            free_tbl_node(tbl->nodes[i]);
    }
}

void free_tbl_node(Node *curr_node)
{
    if (curr_node->next != NULL)
        free_tbl_node(curr_node->next);

    free(curr_node->value);
    curr_node->value = NULL;
    free(curr_node->key);
    curr_node->key = NULL;
    //free(curr_node);

    //Node *temp = NULL;
    //Node *temp2 = NULL;

    //temp = temp2 = curr_node;
    //while (temp->next != NULL)
    //{
    //    temp2=temp->next;
    //    free(temp->key);
    //    free(temp->value);
    //    free(temp);
    //    temp=temp2;
    //}
}

// table operations
HASHTABLE_API int count(Hashtable_str *tbl) { return tbl->num_elements; }
HASHTABLE_API int size(Hashtable_str *tbl) { return tbl->size; }

HASHTABLE_API int tbl_insert(Hashtable_str *table, const char * const key, const char * const element)
{
    int hash;
    Node *temp_ptr = NULL;
    hash = table->hash_function(key);

    //    printf("Placing into column %d\n", hash);

    if (table->nodes[hash] == NULL)
    {
        table->nodes[hash] = (Node*) malloc(sizeof(Node*));
        temp_ptr = table->nodes[hash];
        temp_ptr->next = NULL;
        temp_ptr->key = (char*) malloc (strlen(key) + 1 * sizeof(char));
        temp_ptr->value = (char*) malloc (strlen(element) + 1 * sizeof(char));
        strcpy_s(temp_ptr->key, strlen(key)+1, key);
        strcpy_s(temp_ptr->value, strlen(element)+1, element);
        table->num_elements += 1;
    }
    else
    {
        // Collision!!
        temp_ptr = table->nodes[hash];
        while (temp_ptr->next != NULL)
            temp_ptr = temp_ptr->next;

        temp_ptr->next = (Node*) malloc(sizeof(Node));
        temp_ptr->next->key = (char*) malloc (strlen(key)+1 * sizeof(char));
        temp_ptr->next->value = (char*) malloc (strlen(element)+1 * sizeof(char));
        temp_ptr->next->next = NULL;
        strcpy_s(temp_ptr->next->key, strlen(key)+1, key);
        strcpy_s(temp_ptr->next->value, strlen(element)+1, element);
        table->num_elements += 1;
    }

    // Return the hash value itself for hacking
    return hash;
}

HASHTABLE_API int tbl_remove(Hashtable_str *tbl, const char * const key)
{
    int hash;
    Node *temp_ptr = NULL;
    Node *chain = NULL;
    hash = tbl->hash_function(key);

    if (tbl->nodes[hash] == NULL)
        return 1;
    else
    {
        temp_ptr = tbl->nodes[hash];

        if (strcmp(key, temp_ptr->key) == 0)
        {
            // The next node is the node in question
            chain = temp_ptr->next;
            free(temp_ptr->value);
            printf("Deleted the value\n");
            free(temp_ptr->key);
            printf("Deleted the key\n");
            //printf("About to delete the node itself\n");
            //free(temp_ptr);
            tbl->nodes[hash] = chain;
            tbl->num_elements -= 1;
            return 0;
        }
        else
        {
            while (temp_ptr->next != NULL)
            {
                if (strcmp(key, temp_ptr->next->key) == 0)
                {
                    // The next node is the node in question
                    // So grab a pointer to the node after it
                    // and remove the next node
                    chain = temp_ptr->next->next;
                    free(temp_ptr->next->key);
                    free(temp_ptr->next->value);
                    //free(temp_ptr->next);
                    temp_ptr->next = chain;
                    tbl->num_elements -= 1;
                    return 0;
                }
                else
                    temp_ptr = temp_ptr->next;
            }
        }

        // Couldn't find the node, so declare not existent
        return 1;
    }
}

HASHTABLE_API char * tbl_find(Hashtable_str *tbl, const char * const key)
{
    // Compute the hash for the index
    int hash;
    Node *temp_ptr = NULL;
    hash = tbl->hash_function(key);

    if (tbl->nodes[hash] == NULL)
        return NULL;
    else
    {
        temp_ptr = tbl->nodes[hash];

        if (strcmp(key, temp_ptr->key) != 0)
        {
            while (temp_ptr->next != NULL)
            {
                temp_ptr = temp_ptr->next;
                if (strcmp(key, temp_ptr->key) == 0)
                    return temp_ptr->value;
            }
        }

        // Couldn't find the node, so declare not existent
        return NULL;
    }
}

Here's my main

#include <hashtable.h>
#include <utils.h>
#include <stdio.h>

int main(int argc, char **argv)
{
    int i=0;
    Hashtable_str *my_table = NULL;
    tbl_construct(&my_table);

    tbl_insert(my_table, "Daniel", "Student");
    tbl_insert(my_table, "Derek", "Lecturer");
    //tbl_insert(my_table, "Melvyn", "Lecturer");

    tbl_print(my_table);

    printf("\nRemoving Daniel...\n");
    tbl_remove(my_table, "Daniel");
    //tbl_print(my_table);

    tbl_destruct(my_table);
    my_table = NULL;

    scanf_s("%d", &i);
    return 0;
}
Lancophone
  • 310
  • 2
  • 17
  • You have multiple calls to free in your code. It would help if you pointed out which one is causing the error. And tell us what th error is. – Paul Mitchell Apr 20 '12 at 11:04
  • @Lancophon Also you don't need to paste your entire program, only the parts relevant to your question. – sashoalm Apr 20 '12 at 11:05
  • @PaulMitchell Apologies for being unclear. The erroneous call is in the tbl_remove() function, just before the "deleted key" printf statement – Lancophone Apr 20 '12 at 11:10
  • @satuon Because the hashtable is implemented in a dll, and the main in an executable and my problem is with regards linking the dll and memory errors, I felt the entire program was relevant – Lancophone Apr 20 '12 at 11:12

1 Answers1

1

This is incorrect as allocates the size of a pointer, not a Hashtable_str:

*tbl = (Hashtable_str*) malloc (sizeof(Hashtable_str*));

it should be:

*tbl = malloc(sizeof(Hashtable_str));

Same issue with:

table->nodes[hash] = (Node*) malloc(sizeof(Node*));

See Do I cast the result of malloc?

Community
  • 1
  • 1
hmjd
  • 120,187
  • 20
  • 207
  • 252
  • Thank you very much for your reply. I have made your modification (apologies for the slip on my part). However, this has not remedied the crash – Lancophone Apr 20 '12 at 11:16
  • That update fixed it!!! Thank you so much!! However, this raises another question; Why does this code compile and execute fine in Linux with gcc?? – Lancophone Apr 20 '12 at 11:21
  • @Lancophon, accessing unallocated memory is undefined behaviour, meaning anything can happen. Unluckily, it "worked" on Linux. – hmjd Apr 20 '12 at 11:22
  • The most ironic negative luck I've ever experienced :-) Thank you very much again, I was beginning to give up on this problem – Lancophone Apr 20 '12 at 11:27