1

I am working on an university assignment. The general idea is to make a library, written in c++, and expose its content in both C and C++ headers. We are supposed to have a global variable containing dictionary (unordered_map) of dictionaries storing telephone number changes and modify it with functions exposed in header. In other words we have unordered_map<id, unordered_map<old_phone_nr, new_phone_number>>. Here is my C++ header

namespace jnp1 {

extern "C" {
extern size_t TEL_NUM_MAX_LEN;

unsigned long maptel_create();
// create a dictionary of telephone number changes (old->new) and return its ID

void maptel_delete(unsigned long id);
// delete the dictionary ID

void maptel_insert(unsigned long id, char const *tel_src, char const *tel_dst);
// insert change into dictionary ID

void maptel_erase(unsigned long id, char const *tel_src);
// erase change from dictionary ID

void maptel_transform(unsigned long id, char const *tel_src, char *tel_dst, size_t len);
// follow chain of changes (nr1 -> nr2 -> ... -> final_number) and return final number
}
}

And we have a similar C header. Our library has to conform to external tests - C version:

#include <assert.h>
#include <string.h>
#include "maptel.h" // our library C header

static const char t112[] = "112";
static const char t997[] = "997";

int main() {
  unsigned long id;
  char tel[TEL_NUM_MAX_LEN + 1]; /* +1 for terminal zero */

  id = maptel_create();
  maptel_insert(id, t112, t997);
  maptel_transform(id, t112, tel, TEL_NUM_MAX_LEN + 1);
  assert(strcmp(tel, t997) == 0);
return 0;

}

this works without any problems. However the C++ test case uses unnamed namespace like that

#include <cassert>
#include <cstddef>
#include <cstring>
#include "cmaptel" // our library C++ header

namespace {

  unsigned long testuj() {
    unsigned long id;

    id = ::jnp1::maptel_create();
    ::jnp1::maptel_insert(id, "997", "112");

    return id;
  }

  unsigned long id = testuj();

} // anonymous namespace

int main() {
  char tel[::jnp1::TEL_NUM_MAX_LEN + 1];

  ::jnp1::maptel_transform(id, "997", tel, ::jnp1::TEL_NUM_MAX_LEN + 1); // here it breaks
  assert(::std::strcmp(tel, "112") == 0);
  ::jnp1::maptel_delete(id);
}

On first access from outside the namespace an assertion we made inside the function breaks, because id is not in the dictionary. So, in other words, the anonymously namespaced maptel_create() creates the id dictionary in some other instance of the variable than the maptel_transform() in void main(){} tries to access it. We were not able to troubleshoot this issue, however moving our global unordered_map<unsigned long, unordered_map<string, string>> to heap (we made the variable a pointer, not an object, and allocated it in first call of maptel_create()) works - except specification of the task has no guarantee we can deallocate it, resulting in a memory leak.

I am looking for help/explanation of this phenomenon. Please don't tell me to use a class instead of global variables - while this is obviously the correct design choice, we have a specification we must conform to. Below I show heap and stack versions of the create/erase functions (in this matter erase works same as transform, it's just shorter so better as an example)

Stack:

#include "cmaptel"

typedef std::unordered_map<std::string, std::string> tel_book;
std::unordered_map<unsigned long, tel_book> books;

unsigned long id_counter;

extern "C" {
    size_t TEL_NUM_MAX_LEN = 22;

    unsigned long jnp1::maptel_create()
    {
        books.reserve(1);
        // without this for some reason we would get float arithmetic exception right here

        books[id_counter] = tel_book();
        return id_counter++;
    }

    void jnp1::maptel_erase(unsigned long id, char const *tel_src)
    {
        assert(books.count(id)); // this is the assertion that breaks
        std::string src(tel_src);
        int result = books[id].erase(src);

    }
}

Heap:

#include "cmaptel"

typedef std::unordered_map<std::string, std::string> tel_book;
std::unordered_map<unsigned long, tel_book> *books;

unsigned long id_counter;

extern "C" {
    size_t TEL_NUM_MAX_LEN = 22;

    unsigned long jnp1::maptel_create()
    {
        if (books == NULL) books = new std::unordered_map<unsigned long, tel_book>();
        (*books)[id_counter] = tel_book();
        return id_counter++;
    }

    void jnp1::maptel_erase(unsigned long id, char const *tel_src)
    {
        assert(books->count(id));
        std::string src(tel_src);
        int result = (*books)[id].erase(src);

    }
}
Jatentaki
  • 11,804
  • 4
  • 41
  • 37

1 Answers1

1

Initialization order for globals is not specified across multiple translation units. The problem is that id (and thus the call to maptel_insert) is being initialized before books. Since maptel_insert is operating on an uninitialized object, this is undefined behavior.

You've found one way to solve it: allocate the map dynamically on-demand, thus ensuring it gets allocated before use. Another slight variation would be to use a function-scoped static, as these have rules guaranteeing they are initialized before use:

std::unordered_map<unsigned long, tel_book>& books()
{
    static std::unordered_map<unsigned long, tel_book> instance;
    return instance;
}

C++11 also guarantees this initialization is done in a thread-safe manner.

Community
  • 1
  • 1
Dark Falcon
  • 43,592
  • 5
  • 83
  • 98