Error trying to find const char* key from std::map

Question

I have a map declared like this:

std::map<const char*, const char*> map2D;

The map is filled by the output from an API function, which returns const char*:

map2D.insert(std::pair<const char*, const char*>(TempA, TempB));

Now there are 50 TempA values and 50 TempB values, and there is one key with the name of "months". When I am searching for this key, I am getting "not found". E.g.:

std::map<const char*, const char*>::iterator it;
it = map2D.find("months");

if (it != map2D.end()) 
{
    std::cout << "Found " << it->first << " " << it->second << '\n';
}
else 
{
    std::cout << "Not found\n";
}

But when I am doing it like this:

map2D.insert(std::pair<const char*, const char*>("months", "June");

I can find the respective month. After searching the web, I understand that this problem may be related to the use of const char*. Can anyone further clarify this?

Don't map `char const*` (or `char *`, etc.) unless you seriously know **why** you're doing it. (and this isn't one of those times). Use `std::map`. I doubt you're trying to map `char` addresses, at least not intentionally. — WhozCraig, Jun 25 '14 at 01:29
A bit suprising that it even works with your second example - compilers really do lots of work on our code... — j_kubik, Jun 25 '14 at 01:32
@WhozCraig...as i mentioned i am filling the map with a output from an API function and that function outputs only `const char*`, so if i use `std::map`, what is recommended, casting or conversion before filling the map? — Hamid, Jun 25 '14 at 01:42
@WhozCraig And btw, `std::map` has the ability for custom comparator, so using std::string is unnecesary. String class implementation will usually insist on copying the buffer upon creation, so if his external API promises not to free the buffer as long as he uses it, using `std::string` would be suboptimal. — j_kubik, Jun 25 '14 at 01:48
And, btw2: If using `std::string` at all, then `std::map` would suffice, wouldn't it? :) — j_kubik, Jun 25 '14 at 01:50
@j_kubik: It's not just about ordering; it's about ownership. Storing `const char*` in a container should be _exceedingly_ rare and utterly plastered with explanatory comments, promising that it's safe and detailing precisely why this is so. From what Hamid has said, it's far more likely that he should be constructing `std::string`s from his API's output, but has simply not considered that this is possible. — Lightness Races in Orbit, Jun 25 '14 at 01:52
@LightnessRacesinOrbit - about ownership? I wrote "if his external API promises not to free the buffer as long as he uses it" - well I meant either free it or change it's contents. Though it's possible to store pointers as map keys and free them on map destruction manually, I was rather recommending using it provided that pointed entities are guaranteed to outlive the map itself but are also guaranteed to be destroyed by some other means - thus avoiding entire ownership hassle. — j_kubik, Jun 25 '14 at 02:13
possible duplicate of [Using char\* as a key in std::map](http://stackoverflow.com/questions/4157687/using-char-as-a-key-in-stdmap) — Pradhan, Jun 25 '14 at 02:22
@j_kubik do you see a custom comparator in the OP's code? I'm fully aware one can be provided. The OP's hasn't one, and providing one isn't the solution to *anything* as far as the OP's code is concerned. That was the *point*. They are clearly new to `std::map<>`, and still operating in a C-world while doing so. My comment, "unless you seriously know why you're doing it" specifically does *not* rule out *ever* mapping char pointers as keys, and that *was* intentional. — WhozCraig, Jun 25 '14 at 02:56
@WhozCraig I know. If I were writing an answer, I would probaly also recomend using std::string, but my comment I adressed more to other readers that will answer this question (like you), rather than OP. In my opinion under certain circumstances, using custom comparator rather than std::string might be beneficial. Perhaps I shouldn't deviate from answering OP into open discussion this much... — j_kubik, Jun 25 '14 at 03:02
@j_kubik certainly. there are truly times it comes in handy, rare as they are, to do *exactly* as you describe, (and I admit I've done them, ). It isn't often, but when it is needed its nice to know its there for the taking. The [linked answer](http://stackoverflow.com/questions/4157687/using-char-as-a-key-in-stdmap) Pradhan provided describes exactly what you refer to and the pitfalls of what happens when you don't do it. Pretty good reference. — WhozCraig, Jun 25 '14 at 03:16
@DavidSchwartz: Except that, depending on your compiler and your settings, that might be always true, or always false, or, most likely, be true every time you're running in production and false every time you're debugging. :) So, you may not actually see the problem. (IIRC, with default settings in at least g++ and clang++, it'll always be true, and will in fact warn you that it's always true, which would be the exact wrong message to take away…) — abarnert, Jun 25 '14 at 04:32

Lightness Races in Orbit · Accepted Answer · 2014-06-25T02:05:54.600

11

Comparing two const char* for equality does not do what you think it does; it compares pointer values, not the strings that the pointers point to. With string literals this may occasionally "work", but you have no way of knowing that two string literals even with the same characters in it will be stored at the same address. You would have to provide a custom comparator that invokes strcmp, in order to make that work reliably.

You're much better off with a std::map<std::string, std::string>. It doesn't matter that your third-party API gives you const char*: you can simply construct std::strings from those.

This container will have elements with clear ownership and lifetime semantics, and be automatically ordered properly. In short, all your problems will simply go away.

If you still really need to store const char*, be aware that such a requirement is exceedingly rare and should only be even fleetingly considered if you are prepared to litter your code with explanatory comments detailing precisely how and why your container is safe; hint: it almost certainly is not.

edited Jun 25 '14 at 02:05

answered Jun 25 '14 at 01:54

Lightness Races in Orbit

378,754
76
643
1,055

I had this idea of using `std::map`..now i m using string map and filling item like this `map2D(std::pair(static_cast (TempA),static_cast (TempB))`..do you think this is acceptable and trust worthy way to do? – Hamid Jun 25 '14 at 02:27
3

@Hamid no need for the casts. `std::string` will construct automagically (at least as far as you're concerned) in the case you mention. In fact, you could simply `map2D[TempA] = TempB;` and the operators and constructors would take care of the specifics for you. That in itself is likely worth you playing around in the language and reading up on. There is a *ton* going on in that seemingly innocent looking statement. Best of luck. – WhozCraig Jun 25 '14 at 03:11
1

Oh, and +1, Lightness. Nice answer (as usual). – WhozCraig Jun 25 '14 at 03:13
TempA and TempB are 3rd Party API function which outputs `const char*`. – Hamid Jun 25 '14 at 03:25
@Hamid: Then you certainly can't cast those functions themselves to `string`, which is what your code attempts to do. You can cast the result of _calling_ those functions to `string`. But you can still do that the way WhozCraig showed: `map2D[TempA()] = TempB();`. – abarnert Jun 25 '14 at 04:35
I tried to do the `map2D[TempA()]=TempB();` but then `map2D.find()` is giving "Not Found" message. – Hamid Jun 25 '14 at 04:53

score 0 · Answer 2 · answered Dec 29 '21 at 11:00

The built-in comparison operator for const char* compares the pointer address, not the strings it points to:

std::map<const char*, const char*> map2D; //don't use that!
map2D.emplace("a", "b");
//...
auto key = std::string{"a"};
assert(map2D.find(key.c_str()) == map2D.end()); //not found

If you create a map from string literals (which you can assume the lifetime of a program) use C++17 std::string_view instead of const char* as the key. This class has custom operator= which does the job in a way you'd expect when using algorithms:

std::map<std::string_view, const char*> map2D;
map2D.emplace("a", "b");
//...
auto key = std::string{"a"};
assert(map2D.find(key)->second == std::string_view{"b"});

On the other hand, if you can't say nothing about the lifetime of the string pointed by const char*, use std::string which will make a deep copy. It is less-performant but more general solution:

std::map<std::string, const char*> map2D;
map2D.emplace("a", "b");
//...
auto key = std::string{"a"};
assert(map2D.find(key)->second == std::string_view{"b"});

Error trying to find const char* key from std::map

2 Answers2

Linked

Related