2

Here's my code...

char* m1;
char* m2 = "sth";
strcpy(m1, m2);

That code threw run-time errors, so I tried...

char m1[];
char m2[] = "sth";

That can run with no errors. However, I want to use...

char* s

What sould I do?

Andrew Brēza
  • 7,705
  • 3
  • 34
  • 40
barssala
  • 463
  • 2
  • 6
  • 22
  • 3
    `strcpy` requires enough space at the destination. Any reason you can't use `std::string`? – BoBTFish Sep 17 '12 at 08:57
  • I would recommed.. char *m1 = malloc( sizof(char) * (strlen(m2) + 1)); and declare m2 first. then you dont need to worry about magic numbers. – MarsRover Sep 17 '12 at 09:00
  • @MarsRover: `sizeof(char)` is redundant as it is always 1 by definition. – Paul R Sep 17 '12 at 09:01
  • I like putting in the sizeof(char) as it gets optimized out and improves code readability – Minion91 Sep 17 '12 at 09:02
  • @Minion91: you could make a case for putting `sizeof(*m1)`, but `sizeof(char)` is just needless clutter. – Paul R Sep 17 '12 at 09:04
  • @PaulR I disagree, sizeof(char) indicates the type of the malloced resource – Minion91 Sep 17 '12 at 09:06
  • @Minion91: adding redundant information that the compiler ignores but that educates readers is what comments are for :-) Anyway, this is C++, MarsRover's code doesn't compile because `malloc` returns `void*`, which doesn't convert to `char*` implicitly in C++. Once fixed, the cast would indicate the type for a *third* time on one line, how much indication do you think the reader needs before they believe you that you wanted some `char`s? – Steve Jessop Sep 17 '12 at 09:23
  • @Minion91: it is better to use `sizeof(*m1)` so that if the type should change in the future (e.g. say it changes to `wchar_t`) you only need to make the change in one place - had you used `sizeof(char)` then you would need to change it in two places, and then guess what might happen ? ;-) This is known as the SPOT (Single Point Of Truth) principle. – Paul R Sep 17 '12 at 09:27
  • @PaulR: well, in C++03 you still have to change it in two places (the definition of the pointer `m1` and the cast of the result of `malloc`). In C++11 you could do that cast by `static_cast` in order that one place specifies the type, and everything else follows. Or you could use something a bit more C++-friendly than `malloc`. – Steve Jessop Sep 17 '12 at 09:29
  • @Steve: yes, but at least the compiler will catch the mistake if you forget to change the type of the cast, whereas forgetting to change `sizeof(char)` in the call to malloc will compile without warning. – Paul R Sep 17 '12 at 09:33

5 Answers5

2

You should allocate memory for m1. For example

char* m1 = new char[4];
char* m2 = "sth";
strcpy(m1, m2);
// so smth with m1
delete[] m1;

But since you write on C++, why you not use std::string?

ForEveR
  • 55,233
  • 2
  • 119
  • 133
  • The last sentence (of an answer!) shouldn't be a question although the intent of the (rethorical) question is clear. – TobiMcNamobi Nov 16 '17 at 14:33
1

Problem is you are not allocating any memory to store the result of the strcpy method. You need:

char* m1 = (char *) malloc(sizeof(char) * 4) /* 4 being the size of 3 chars + trailing \0 */
char* m2 = "sth";

Than you can do the strcpy

And in the end you have to free any dynamically allocated memory a.g.

free(m1)

m2 is allocated statically and doesn't has to be freed.

Minion91
  • 1,911
  • 12
  • 19
1

In C++ you would rather do:

// don't forget to #include <string>
std::string m1;
std::string m2 = "sth";
m1 = m2;

if you then need a const char* (e.g., for some API call) you can get one

const char* str = m1.c_str();

Problem gone.

Plus, you don't need to bother with buffer sizes and proper deallocation anymore.

moooeeeep
  • 31,622
  • 22
  • 98
  • 187
0

I believe even your 2nd example is also invalid

char m1[];
char m2[] = "sth";

m1 is not having any valid piece of memory to store the copied data. And, I believe you should have the array dimension in your declaration (you are not declaring a method param here)

Normally you should have something like 

char m1[10];
char m2[] = "sth"; 
strcpy(m1, m2);

Go back to the question. Many people said you should allocate memory in order to use char*. It is not wrong, but more accurately, you should have the pointer pointing to valid piece of memory.

Allocating memory is one way, but you can also do something like this:

char outdata[100];
char* m1 = &(outdata[10]);
char* m2 = "foo";
strcpy(m1, m2);

no explicit allocation here, and still a valid example.

I believe OP is still not understanding what is an array and what is a pointer. Get some extra reading and thinking on that.

Adrian Shum
  • 38,812
  • 10
  • 83
  • 131
-2

First of all, before you perform the copy you must assure that the destination pointer have enough memory to keep the copy, if you don't you'll get runtime errors. So you need this:

char* m1;
char* m2 = "sth";
m1 = new char[strlen(m2)+1]; // <---- reserve memory for m1
strcpy(m1, m2);
// remember to delete[] m1

Another way to deal with character strings is to use the std::string class instead, if you do, the memory management will not bother you.

std::string m1("");
std::string m2("sth");
m1 = m2; // no strcopy needed, no memory management needed

Finally, if for some reason you must deal with pointers, take care of the new/delete pair and remenber that the memory reserved with new[] must be deleted wit delete[].

And and additional advice: instead of memcpy i preffer the use of std::copy:

char* m1;
char* m2 = "sth";
m1 = new char[strlen(m2)+1]; // <---- reserve memory for m1
std::copy(std::begin(m2), std::end(m2), std::begin(m1));
// remember to delete[] m1

Because it works both for the char * and the std::string version if you use std::begin and std::end functions:

std::string m1("");
std::string m2("sth");
std::copy(std::begin(m2), std::end(m2), std::begin(m1));

Edit: It works as long the m2 are statically reserved so the length can be deduced at compile time.

Community
  • 1
  • 1
PaperBirdMaster
  • 12,806
  • 9
  • 48
  • 94