2

I have this code at the moment and am getting an output that I just cant understand.

This is my symTable module, which has all the functions needed to manipulate a symbol table.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <ctype.h>
#include <errno.h>
#include <assert.h>
#include "symTable.h"
#define HASH_MULTIPLIER 65599
#define SIZE 61

struct SymTable {
    char *key;
    void *value;
    struct SymTable *next;
};

/*--------------------------------------------------------------------*/

/* Hash string str to a hash table bucket index in range [0, htsize] */

static unsigned int hash(const char *key, const int htsize)
{
    int i;
    unsigned int h = 0U;
    for (i = 0; key[i] != '\0'; i++)
        h = h * HASH_MULTIPLIER + (unsigned char) key[i];
    return h % htsize;
}

SymTable_T symTable_create(void)
{


    SymTable_T p;
    p = calloc(SIZE, sizeof(SymTable_T));
    p->key = (char*)calloc(1, sizeof(char));;

    return p;
}


void symTable_destroy(SymTable_T symTable)
{


    SymTable_T p, nextp;
    int i;

    assert(symTable != NULL);

    for (i = 0; i < SIZE; i++) {
        for (p = symTable; p != NULL; p = nextp) {
            nextp = p->next;
            assert(p->key != NULL);
            p=NULL;
        }
    }
    free(symTable);
    return;

}

int symTable_size(SymTable_T symTable)
{

    SymTable_T p, nextp;
    int i=0;
    assert(symTable != NULL);
    for (p = symTable; p != NULL; p = nextp) {
        nextp = p->next;
        i++;
    }

    return i;

}

int symTable_insert(SymTable_T symTable, const char *key,
                    const void *value, size_t valuesize)
{

    SymTable_T p, prevp = NULL;
    int h;


    assert(symTable != NULL);

    if (key == NULL)
        return 0;                   // insert failed: return 0

    /* Get hash of key*/
    h = hash(key, SIZE);


    for (p = symTable; p != NULL; prevp = p, p = p->next) {

        if (hash(p->key, SIZE) == h)
        {
            return 0;
        }

    }

    p = (SymTable_T)malloc(sizeof(SymTable_T));
    if (p == NULL)
        return 0;
    p->key =(char*) key;
    p->value = (void*)calloc(1, valuesize);
    memcpy(p->value, value, valuesize);
    p->next = NULL;
    if (symTable == NULL)
        symTable = p;
    else
        prevp->next = p;
    return 1;
}

int symTable_search(SymTable_T symTable, const char *key,
                    void *value, size_t valuesize)
{

    SymTable_T p;
    int i;
    assert(symTable != NULL);
    for (i = 0; i < SIZE; i++) {

        for (p = symTable->next; p != NULL; p = p->next) {
            if (strcmp(p->key, key) == 0)
            {
                //value = (void*)calloc(1, valuesize);
                memcpy(value, p->value, valuesize);
                //  printf("%c"()
                return 1;
            }

        }
    }
    return 0;

}

int symTable_delete(SymTable_T symTable, const char *key)
{
    SymTable_T p, nextp;
    int i;
    assert(symTable != NULL);
    for (i = 0; i < SIZE; i++) {
        for (p = symTable->next; p != NULL; p = nextp) {
            nextp = p->next;

            assert(p->key != NULL);
            if (strcmp(p->key, key) == 0)
            {

                p = NULL;
                return 1;
            }
        }
    }
    return 0;
}

I have a header and a C client file to run the program.

Main client file is:

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

int main(int argc, char *argv[])
{
    char *key[4] = { "Einstein", "Newton", "Bohr", "Curie" };
    char cvalue, char_value[4] = { 'e', 'n', 'b', 'c' };
    int  ivalue, int_value[4] = { 10, 20, 30, 40 };
    float fvalue, float_value[4] = { 1.11, 2.22, 3.33, 4.44 };
    double darray[2], double_array[4][2] = { { 1.11, 2.22 },
                    { 3.33, 4.44 }, { 5.55, 6.66 }, { 7.77, 8.88 } };

    int i, rv;

    SymTable_T charTable, intTable, floatTable, double_arrayTable;

    /* Create and insert character values into symbol table. */
    printf("\nCreating symbol table with character values\n");
    printf("-------------------------------------------\n");
    charTable = symTable_create();
    if (charTable == NULL) {
        fprintf(stderr, "Cannot create character table\n");
        return 1;
    }
    for (i = 0; i < 4; i++) {
        printf("Insert <%s, %c>\n", key[i], char_value[i]);
        rv = symTable_insert(charTable, key[i], &char_value[i], sizeof(char));
        if (!rv) {
            fprintf(stderr, "Insert failed for %s\n", key[i]);
            return 1;
        }
    }

    /* Create and insert integer values into symbol table. */
    printf("\nCreating symbol table with integer values\n");
    printf("-----------------------------------------\n");
    intTable = symTable_create();
    if (intTable == NULL) {
        fprintf(stderr, "Cannot create integer table\n");
        return 1;
    }
    for (i = 0; i < 4; i++) {
        printf("Insert <%s, %d>\n", key[i], int_value[i]);
        rv = symTable_insert(intTable, key[i], &int_value[i], sizeof(int));
        if (!rv) {
            fprintf(stderr, "Insert failed for %s\n", key[i]);
            return 1;
        }
    }

    /* Create and insert float values into symbol table. */
    printf("\nCreating symbol table with float values\n");
    printf("---------------------------------------\n");
    floatTable = symTable_create();
    if (floatTable == NULL) {
        fprintf(stderr, "Cannot create float table\n");
        return 1;
    }
    for (i = 0; i < 4; i++) {
        printf("Insert <%s, %g>\n", key[i], float_value[i]);
        rv = symTable_insert(floatTable, key[i], &float_value[i], sizeof(float));
        if (!rv) {
            fprintf(stderr, "Insert failed for %s\n", key[i]);
            return 1;
        }
    }

    /* Create and insert an array two double values into symbol table. */
    printf("\nCreating symbol table with arrays of two double values\n");
    printf("------------------------------------------------------\n");
    double_arrayTable = symTable_create();
    if (double_arrayTable == NULL) {
        fprintf(stderr, "Cannot create double array table\n");
        return 1;
    }
    for (i = 0; i < 4; i++) {
        printf("Insert <%s, [%g, %g]>\n", key[i], double_array[i][0], double_array[i][1]);
        rv = symTable_insert(double_arrayTable, key[i], &double_array[i][0], 2*sizeof(double));
        if (!rv) {
            fprintf(stderr, "Insert failed for %s\n", key[i]);
            return 1;
        }
    }

    /* Search and print <key, value> pairs in character table, then delete */
    printf("\nSearching symbol table with character values\n");
    printf("--------------------------------------------\n");
    for (i = 0; i < 4; i++) {
        rv = symTable_search(charTable, key[i], &cvalue, sizeof(char));
        if (rv) {
            printf("%s found, value = %c: now deleting\n", key[i], cvalue);
            if (!symTable_delete(charTable, key[i])) {
                fprintf(stderr, "Delete failed for %s\n", key[i]);
                return 1;
            }
        }
        else  {
            fprintf(stderr, "Search failed for %s\n", key[i]);
            return 1;
        }
    }

    /* Search and print <key, value> pairs in integer table, then delete */
    printf("\nSearching symbol table with integer values\n");
    printf("------------------------------------------\n");
    for (i = 0; i < 4; i++) {
        rv = symTable_search(intTable, key[i], &ivalue, sizeof(int));
        if (rv) {
            printf("%s found, value = %d: now deleting\n", key[i], ivalue);
            if (!symTable_delete(intTable, key[i])) {
                fprintf(stderr, "Delete failed for %s\n", key[i]);
                return 1;
            }
        }
        else  {
            fprintf(stderr, "Search failed for %s\n", key[i]);
            return 1;
        }
    }

    /* Search and print <key, value> pairs in float table, then delete */
    printf("\nSearching symbol table with float values\n");
    printf("----------------------------------------\n");
    for (i = 0; i < 4; i++) {
        rv = symTable_search(floatTable, key[i], &fvalue, sizeof(float));
        if (rv) {
            printf("%s found, value = %g: now deleting\n", key[i], fvalue);
            if (!symTable_delete(floatTable, key[i])) {
                fprintf(stderr, "Delete failed for %s\n", key[i]);
                return 1;
            }
        }
        else  {
            fprintf(stderr, "Search failed for %s\n", key[i]);
            return 1;
        }
    }

    /* Search and print <key, value> pairs in double array table, then delete */
    printf("\nSearching symbol table with arrays of two double values\n");
    printf("-------------------------------------------------------\n");
    for (i = 0; i < 4; i++) {
        rv = symTable_search(double_arrayTable, key[i], &darray, 2*sizeof(double));
        if (rv) {
            printf("%s found, value = [%g, %g]: now deleting\n", key[i], darray[0], darray[1]);
            if (!symTable_delete(double_arrayTable, key[i])) {
                fprintf(stderr, "Delete failed for %s\n", key[i]);
                return 1;
            }
        }
        else  {
            fprintf(stderr, "Search failed for %s\n", key[i]);
            return 1;
        }
    }

    symTable_destroy(charTable);
    symTable_destroy(intTable);
    symTable_destroy(floatTable);
    symTable_destroy(double_arrayTable);
    return 0;
}

The header file also includes this for the structure:

typedef struct SymTable *SymTable_T;

My output currently is this, and as you can see, the values are completely wrong and I am unsure as to why. I think the problem is in my insert function regarding the memory, but I'm unsure:

My output:

Creating symbol table with character values
-------------------------------------------
Insert <Einstein, e>
Insert <Newton, n>
Insert <Bohr, b>
Insert <Curie, c>

Creating symbol table with integer values
-----------------------------------------
Insert <Einstein, 10>
Insert <Newton, 20>
Insert <Bohr, 30>
Insert <Curie, 40>

Creating symbol table with float values
---------------------------------------
Insert <Einstein, 1.11>
Insert <Newton, 2.22>
Insert <Bohr, 3.33>
Insert <Curie, 4.44>

Creating symbol table with arrays of two double values
------------------------------------------------------
Insert <Einstein, [1.11, 2.22]>
Insert <Newton, [3.33, 4.44]>
Insert <Bohr, [5.55, 6.66]>
Insert <Curie, [7.77, 8.88]>

Searching symbol table with character values
--------------------------------------------
Einstein found, value = \300: now deleting
Newton found, value = \340: now deleting
Bohr found, value = : now deleting
Curie found, value = : now deleting

Searching symbol table with integer values
------------------------------------------
Einstein found, value = 1070416: now deleting
Newton found, value = 1070448: now deleting
Bohr found, value = 1071472: now deleting
Curie found, value = 0: now deleting

Searching symbol table with float values
----------------------------------------
Einstein found, value = 1.50248e-39: now deleting
Newton found, value = 1.50253e-39: now deleting
Bohr found, value = 1.50257e-39: now deleting
Curie found, value = 0: now deleting

Searching symbol table with arrays of two double values
-------------------------------------------------------
Einstein found, value = [2.12253e-314, 2.22]: now deleting
Newton found, value = [2.12253e-314, 4.44]: now deleting
Bohr found, value = [2.12253e-314, 6.66]: now deleting
Curie found, value = [0, 8.88]: now deleting

The correct output should be this:

Creating symbol table with character values
-------------------------------------------
Insert <Einstein, e>
Insert <Newton, n>
Insert <Bohr, b>
Insert <Curie, c>

Creating symbol table with integer values
-----------------------------------------
Insert <Einstein, 10>
Insert <Newton, 20>
Insert <Bohr, 30>
Insert <Curie, 40>

Creating symbol table with float values
---------------------------------------
Insert <Einstein, 1.11>
Insert <Newton, 2.22>
Insert <Bohr, 3.33>
Insert <Curie, 4.44>

Creating symbol table with arrays of two double values
------------------------------------------------------
Insert <Einstein, [1.11, 2.22]>
Insert <Newton, [3.33, 4.44]>
Insert <Bohr, [5.55, 6.66]>
Insert <Curie, [7.77, 8.88]>

Searching symbol table with character values
--------------------------------------------
Einstein found, value = e: now deleting
Newton found, value = n: now deleting
Bohr found, value = b: now deleting
Curie found, value = c: now deleting

Searching symbol table with integer values
------------------------------------------
Einstein found, value = 10: now deleting
Newton found, value = 20: now deleting
Bohr found, value = 30: now deleting
Curie found, value = 40: now deleting

Searching symbol table with float values
----------------------------------------
Einstein found, value = 1.11: now deleting
Newton found, value = 2.22: now deleting
Bohr found, value = 3.33: now deleting
Curie found, value = 4.44: now deleting

Searching symbol table with arrays of two double values
-------------------------------------------------------
Einstein found, value = [1.11, 2.22]: now deleting
Newton found, value = [3.33, 4.44]: now deleting
Bohr found, value = [5.55, 6.66]: now deleting
Curie found, value = [7.77, 8.88]: now deleting

Any help would be appreciated.

Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
  • 1
    It would be helpful if you could provide a minimal example ... – zeeMonkeez Dec 01 '15 at 01:29
  • 1
    Please try to be more concise.. Too much information just for one quesrion.. – rcmgleite Dec 01 '15 at 01:30
  • my issue i think is in the insert function in the first c module. im not sure how i could make the example smaller – Josh Friedman Dec 01 '15 at 01:31
  • i have updated the question – Josh Friedman Dec 01 '15 at 01:35
  • 1
    If you can't make a minimal example at least make a complete one. That is, include a `main` that calls your functions and reproduces the output you have shown. – kaylum Dec 01 '15 at 02:17
  • @kaylum edited with the main file. sorry its alot – Josh Friedman Dec 01 '15 at 02:22
  • 1
    `p = (SymTable_T)malloc(sizeof(SymTable_T));`. That isn't correct. It only allocates a buffer the size of a pointer since `SymTable_T` is a pointer. And best practice in C is not to cast malloc. So should be: `p = malloc(sizeof *p);` or `p = malloc(struct SymTable);` – kaylum Dec 01 '15 at 02:26
  • @kaylum wow, that fixed it thanks man had run out of ideas – Josh Friedman Dec 01 '15 at 02:29
  • You're welcome. A bit of advice. Next time you encounter such problems that look like they could be a result of memory corruption then run your program with [`valgrind`](http://valgrind.org/). Assuming you are developing on a platform that has valgrind support (ie, not Windows). It would have picked up your problem in about 5 seconds. – kaylum Dec 01 '15 at 02:31
  • when calling malloc() or calloc() or realloc(), always check the returned value from 'every' call to assure the call was successful. never use the result of a call to malloc(), etc without first checking for a valid pointer. – user3629249 Dec 01 '15 at 03:35
  • do not use `assert()` in production code as it causes an immediate abort of the program, usually without providing any useful information. When a call to a system function, like `calloc()` returns an error condition, the `perror()` function will output, to stderr, the enclosed text and the errno text message. This is usually followed by cleaning up all allocated memory and then call ing `exit()`, – user3629249 Dec 01 '15 at 03:39
  • the function: `symTable_destory()` is typical of the kinds of problems I see in the posted code. Specifically, the contents of the `symTable` contains (for the scenario) at least 4 malloc'd instances of the `struct symTable` and each instance of the `struct sysTable` contains 3 pointers to allocated memory. Setting a pointer to NULL before passing the pointer to `free()` is a memory leak. Not passing the first two pointers to `free()` is two memory leaks. Suggest re-thinking the functions: `symTable_create()`, `symTable_insert()`, and `symTable_destroy()` – user3629249 Dec 01 '15 at 03:48

1 Answers1

0

p = (SymTable_T)malloc(sizeof(SymTable_T));. That isn't correct. It only allocates a buffer the size of a pointer since SymTable_T is a pointer. And best practice in C is not to cast malloc. So should be: p = malloc(sizeof *p); or p = malloc(struct SymTable); – kaylum

Armali
  • 18,255
  • 14
  • 57
  • 171