shput never fills in "key" field

[jgarvin]

Describe the bug If you put a new entry into a string hash table, then immediately extract it and check the key, it's not filled.

To Reproduce

This tiny program aborts on the abort() call every time I run with v0.67:

#define STB_DS_IMPLEMENTATION
#include "stb_ds.h"

struct data {
    int x;
};

struct table_entry {
    char key[33];
    struct data value;
};

int main()
{
    struct table_entry* table = NULL;
    struct table_entry* w = shgetp_null(table, "test");
    if(!w)
    {
        struct data data;
        memset(&data, 0, sizeof(data));
        shput(table, "test", data);
        w = shgetp_null(table, "test");
        if(!strcmp(w->key, "test") == 0)
        {
            abort();
        }
    }
    return 0;
}

Expected behavior I'd expect the key to be set. Without digging through the implementation I'm surprised the lookup finds anything without the key being set?

[nothings]

shgetp is only meant to be used with shputs

[nothings]

I mean, it's possible there's still a bug even on that path, I didn't test.

[jgarvin]

This has been a point of confusion for me. I'd expect most people want to give a key and get back a pointer to the stored value to avoid copying. But hmget/shget return TV not TV*, that's why I went with shgetp, but shgetp seemed a bit less convenient because it returns T* instead of TV*. But I guess if you only ever use it with shputs then T* and TV* are the same?

[jgarvin]

I've updated it to use shputs but now it won't compile

#define STB_DS_IMPLEMENTATION
#include "stb_ds.h"

struct table_entry {
    char key[33];
    int x;
};

int main()
{
    struct table_entry* table = NULL;
    struct table_entry* w = shgetp_null(table, "test");
    if(!w)
    {
        struct table_entry new_entry;
        memset(&new_entry, 0, sizeof(new_entry));
        strcpy(new_entry.key, "test");
        shputs(table, new_entry);
        w = shgetp_null(table, "test");
        if(!strcmp(w->key, "test") == 0)
        {
            abort();
        }
    }
    return 0;
}

$ gcc -O0 -g3 test.c 
In file included from test.c:2:
stb_bug.c: In function ‘main’:
stb_ds.h:616:33: error: assignment to expression with array type
  616 |      (t)[stbds_temp((t)-1)].key = stbds_temp_key((t)-1)) // above line overwrites whole structure, so must rewrite key here if it was allocated internally
      |                                 ^
stb_ds.h:436:21: note: in expansion of macro ‘stbds_shputs’
  436 | #define shputs      stbds_shputs
      |                     ^~~~~~~~~~~~
stb_bug.c:18:9: note: in expansion of macro ‘shputs’
   18 |         shputs(table, new_entry);
      |         ^~~~~~

Does key need to be char* ? Does shputs require using sh_new_strdup or sh_new_arena?

[nothings]

Yes, shput is a string table, so it needs to be a char*, not a char[].

Using sh_new_strdup or sh_new_arena is not required, but you'll have to manually manage the string storage yourself.

Also, you can't provide a fixed char[] array in the structure and then point to it with a char *key, because the hash table will move around in memory and invalidate the pointer.

[nothings]

You could just use char key[33] with hm_ functions if you always use char key[33] for everything and always 0 out the entire key after the end of the string, but then you can't use string literals.

[jgarvin]

Thanks, got it working. I wasn't sure how the memory management for the key on the item you pass to shputs would work when using strdup/arena but valgrind seems to indicate that that's up to you to handle freeing. This compiles and runs with no leaks or other errors, in case somebody stumbles on this later:

#define STB_DS_IMPLEMENTATION
#include "stb_ds.h"

struct table_entry {
    char* key;
    int x;
};

int main()
{
    struct table_entry* table = NULL;
    sh_new_strdup(table);
    struct table_entry* w = shgetp_null(table, "test");
    if(!w)
    {
        struct table_entry new_entry;
        memset(&new_entry, 0, sizeof(new_entry));
        new_entry.key = malloc(strlen("test")+1);
        strcpy(new_entry.key, "test");
        shputs(table, new_entry);
        free(new_entry.key);
        w = shgetp_null(table, "test");
        if(!strcmp(w->key, "test") == 0)
        {
            abort();
        }
    }
    shfree(table);
    return 0;
}