Search code examples
cprintfglib

Issues when using sprintf to build strings


I have a code that add graphs based on a string label, it's something like:

    struct graph *map = graph_new();

    char *labels[] = { "aa", "ab", "ac", "ad", "ae", "af", "ag", "ah", "ai", 
                        "ba", "bb", "bc", "bd", "be", "bf", "bg", "bh", "bi", 
                        "ca", "cb", "cc", "cd", "ce", "cf", "cg", "ch", "ci", 
                        "da", "db", "dc", "dd", "de", "df", "dg", "dh", "di"};

    for(int i=0; i<36; i++)
        graph_node_add(map, labels[i], i+1);

I'm trying to create the same labels, but instead writing each one, I'm building the string from ascii table as in:

    struct graph *map = graph_new();

    char label[3];

    for(int i=0; i<4; i++) {
        for(int j=0; j<9; j++) {
            sprintf(label, "%c%c", i+97, j+97);
            graph_node_add(map, label, 9*i+j+1);
        }
    }

To me, both snippets looks like the same, but the second one didn't work and I don't know why.

As requested I have added graph_node_add, it uses GLib.

struct node *
graph_node_add(struct graph *graph,
               gchar        *label,
               gint          weight)
{
    struct node * n = g_new(struct node, 1);
    n->neighbors = g_ptr_array_new_full(1, (GDestroyNotify)graph_node_neighbor_destroyed);
    n->weight = weight;
    n->label = g_string_new(label);
    g_hash_table_insert(graph->nodes, label, n);
    return n;
}

Outputs:

First case:

aa(1) -> 
ab(2) -> 
ac(3) -> 
ad(4) -> 
ae(5) -> 
ca(19) -> 
... lot of entries ...
da(28) -> 
db(29) -> 
df(33) -> 
dg(34) -> 
de(32) -> 
di(36) -> 
dh(35) -> 

Second case:

di(1) -> 
di(2) -> 
di(3) -> 
di(4) -> 
di(5) -> 
di(19) -> 
di(20) -> 
di(21) -> 
... lot of entries ...
di(34) -> 
di(32) -> 
di(36) -> 
di(35) -> 

Solution

  • First example:

    graph_node_add(map, labels[i], i+1);
    

    This ends up using labels[i] as the hashtable key.

    Second example:

    graph_node_add(map, label, 9*i+j+1);
    

    This one uses label as the hash table key, the same pointer every time you add a node (hashtable does not make a copy, it uses the pointer you give it). When you modify label contents in the loop, you modify every inserted key as well. If you created a copy of label before adding it to he hashtable, then it should work.

    Something like this maybe:

    struct node *
    graph_node_add(struct graph *graph,
                   gchar        *label,
                   gint          weight)
    {
        struct node * n = g_new(struct node, 1);
        n->neighbors = g_ptr_array_new_full(1, (GDestroyNotify)graph_node_neighbor_destroyed);
        n->weight = weight;
        n->label = g_string_new(label);
        g_hash_table_insert(graph->nodes, g_strdup(label), n);
        return n;
    }
    

    Now you can modify your hash table creation code so you give g_free as the key GDestroyNotify in your g_hash_table_new_full() call.

    Alternatively you could avoid using sprintf and just use g_strdup_printf() instead: then you'd have a new char* on every iteration of loop and the problem would not appear at all -- you'd still need to take care of freeing the char* though.