1

I've been trying to convert a const char to a char for the past 30 minutes. Here's what I've got.

 string atr; 

 getline(cin,atr); // Start off with a string because getline takes nothing else.
 const char *buffA = atr.c_str(); // Create a const char of the string converted to a const char.
 char *buff = ""; // Create a new char to hold the converted result.
 strcat(buff,buffA); // Do the conversion.
 parseargs(buff); // Pass the data on.

However, I get an unhandled exception. I have no idea why. I literally just typed 'try' into the console as my only argument.

Daaksin
  • 834
  • 3
  • 13
  • 28
  • 7
    "I've been trying to convert a const char to a char" NO NO NO NO NO NO. Stop wanting this. Tell us what you are trying to do that you believe would be done by converting a const char to a char. Describe your problem, not merely your solution. – R. Martinho Fernandes Aug 05 '13 at 09:32
  • But why don't you just use the `string` directly ? – Nbr44 Aug 05 '13 at 09:35
  • I recommend that you pick up [a good introductory C++ book (click on this link for a list)](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list) so you can learn some modern C++ techniques that would make your code much more understandable, concise, and safe. As you can see from [nightcracker's answer](http://stackoverflow.com/a/18054812/308661), your code can be rewritten with less lines and actually work. – In silico Aug 05 '13 at 09:38
  • @Nbr44 You can't use 'getline' with a string, unfortunately. – Daaksin Aug 05 '13 at 09:40
  • 4
    @Daaksin: [The C++ standard library provides a version that works with `std::string`s](http://en.cppreference.com/w/cpp/string/basic_string/getline) – In silico Aug 05 '13 at 09:42
  • 1
    @Daaksin but isn't that what you are already doing ? `getline(cin,atr);`. A string is bound to be easier to manipulate than a `char` array, too ! – Nbr44 Aug 05 '13 at 09:43

4 Answers4

8

Try using C++ instead of C idioms:

std::vector<char> data(atr.begin(), atr.end());
data.push_back('\0');
parseargs(&data[0]);
orlp
  • 112,504
  • 36
  • 218
  • 315
  • Close. As written, it's probably undefined behavior: since `parseargs` doesn't take a length, it almost certainly expects a `'\0'` terminated string. So you need to push back a `'\0'` before passing `&data[0]`. (Of course, if `parseargs` doesn't modify its argument, `atr.c_str()` solves the problem completely. It's likely that there's no need for a copy anyway.) – James Kanze Aug 05 '13 at 09:49
  • Straight to the point! Thanks for the code snippet, nightcracker ! – Daaksin Aug 05 '13 at 10:10
5

There are two things wrong with your code. First, you initialize a char* with a string literal. This uses a deprecated convertion; the type of a string literal is char const[] (which converts to char const*, not to char*), because any attempt to modify the literal is undefined behavior. The second is that your string literal is only one char long, so even if you could write to it, unless atr was empty, you're writing beyond the end of the buffer.

You don't tell us anything about parseargs. If it doesn't modify it's argument, you can just pass it atr.c_str(), and be done with it. (If it's a legacy function which ignores const, you may have to use a const_cast here.) If it does modify its argument (say because it uses strtok), then you'll have to explicitly push a '\0' onto the end of atr, and then pass it &atr[0]. (Not a particularly clean solution, but if you're not using atr afterwards, it should work.)

James Kanze
  • 150,581
  • 18
  • 184
  • 329
4

Both your contents of buff and buffA are in read-only memory of the process.
You will actually need to new your buff like

char* buff = new char[32];

This provides memory from the free-store and you can then strcat the string from buffA to buff. You should prefer strncat, though to avoid buffer-overruns and delete your buff eventually.

bash.d
  • 13,029
  • 3
  • 29
  • 42
  • Oh! Right, thank you so much for this answer. I continually forget that in C++ you must assign memory to your variables. Well, anyway, cheers ! – Daaksin Aug 05 '13 at 09:36
  • You are welcome, but please don't Forget to distinguish between heap and stack-based variables (consider, arrays, too)!! – bash.d Aug 05 '13 at 09:37
  • @Daaksin On top of that, it would be a lot better if you just used a `string` instead of converting to `char*`in the first place. – Hulk Aug 05 '13 at 09:40
  • 3
    You definitely don't want to use `strcat` on an uninitialized buffer. And you don't want to use `strncat` if you expect to pass the pointer to function later. If you know the maximum size, you don't want to use `new`; you should just use a local buffer. And if you don't, `std::vector` will do a better job at allocating and freeing the array (and will initialize it so that `strcat` won't be undefined behavior, although why `strcat`, and not `strcpy` beats me). – James Kanze Aug 05 '13 at 09:41
2

This

char *buff = ""; // Create a new char to hold the converted result.

creates a char * that points to (probably read-only) memory of about 1 byte in extent. This:

 strcat(buff,buffA); // Do the conversion.

attempts to overwrite that (probably read-only) memory of 1 or so bytes with an arbitrary string.

The chances are this will promptly crash. If the memory is read only, it will crash immediately. If the memory is not read only it will stomp over random data, resulting in very undefined behaviour.

Why on earth do you want to do that? Does parseArgs actually need a modifiable string? It's parsing arguments, it shouldn't need to change them. If it's really necessary, use a std::vector<char> and pass the address of the first element and hope that all it does is poke the contents of the array, rather than (say) running over the end.

Tom Tanner
  • 9,244
  • 3
  • 33
  • 61