Passing pointer and copy value

Question

I guess I do something stupid here, please help me understand what it is, i get a crash :

char *data="xyz";
int EEIndex=0;
int getEEPROMindex();
void updateEEPROMindex();
void getCmd(char *cmd);
void getcmdAtIndex(int index,char *cmd);


        int main(int argc, const char * argv[]) {

        getCmd(data);
        printf("%s  ",data );
        return 0 ;
    }

        void getCmd(char *cmd)
        {
            getcmdAtIndex(EEIndex, cmd);
         }

        void getcmdAtIndex(int index,char *cmd)
        {
            char *EEPROM[]={"A","E","C","D"};

            strcpy(cmd, EEPROM[index]);
            EEIndex=index+1;

        }

Where does it crash? What is EEIndex? please post a minimal sample that compiles — Ishay Peled, Jul 31 '16 at 11:32
EEIndex at the time of the crash is 0 , it crashes on the line strcpy, without any explanation. — Curnelious, Jul 31 '16 at 11:34
God, people here are something. What is wrong with this question. Dont get this evil. Is the code wrong? is that the right way to copy ? — Curnelious, Jul 31 '16 at 11:36
Don't lose it (; We're merely stating out the fact that without a minimal code sample that actually compiles we can't help. The reason for this is that if I want to help you, I need to see the problem you're seeing and that's impossible if I can't even compile your code — Ishay Peled, Jul 31 '16 at 11:37
@IshayPeled thats the whole code. the only line that is not here is - int main(int argc, const char * argv[]) . — Curnelious, Jul 31 '16 at 11:39
The statement `char *data="xyz";` means you cannot change the contents of `data` it becomes read only. And in you functions you are trying to change its contents that's why it is crashing. — ani627, Jul 31 '16 at 11:41
Possible duplicate of [Program aborts when using strcpy on a char pointer? (Works fine on char array)](http://stackoverflow.com/questions/5645949/program-aborts-when-using-strcpy-on-a-char-pointer-works-fine-on-char-array) — Soner from The Ottoman Empire, Jul 31 '16 at 11:52
@snr and yet the link you gave here, got 9 votes, but mine is wrong for some reason. — Curnelious, Jul 31 '16 at 11:59
@Curnelious - Stack Overflow was very different back then. "Minimal example" wasn't a phrase everyone knew. Anyway, it's just Internet points. I have almost 15,000 and I've had plenty of questions / answers downvoted to hell, sometimes out of a misunderstanding, or because one random person thought it was "off topic" and everyone else bandwagoned. In the long run though, you come out ahead as long as you keep learning and contributing. Don't sweat it. — Andrew Cheong, Jul 31 '16 at 12:09

anatolyg · Accepted Answer · 2016-07-31T11:50:27.257

1

It's customary to make string literals read-only. That is:

char *data="xyz";

when your code tried to change the bytes x, y and z, the OS crashes it.

To make it writable, replace a pointer with an array; you should also specify the size of the array. The easiest method to do it is:

char data[] = "xyz"; // will hold a maximum of 3 bytes (size is implicit)

or

char data[20]; // will hold a maximum of 19 bytes plus an end-of-string byte

You can also use strdup:

char* data;
...
strdup(cmd, EEPROM[index]); // instead of strcpy

but this involves dynamic memory allocation, which I think you don't want to bother with.

edited Jul 31 '16 at 11:50

answered Jul 31 '16 at 11:41

anatolyg

26,506
9
60
134

So the right way to do this, where data should be writeable , is to set data to be larger ? – Curnelious Jul 31 '16 at 11:44
possible to forget `freeing` for like the beginners about `strdup`. He strictly should read beginning C programming for n00bies – Soner from The Ottoman Empire Jul 31 '16 at 11:46
I am not sure I get this thing, although I read about it. you say that char data[]="xyz", will also hold only 3 bytes, so why is it good compared to the pointer ? – Curnelious Jul 31 '16 at 11:54
Thats exactly what the answer state in the comment. Yes there is the null terminator. – Curnelious Jul 31 '16 at 11:56
"why is it good compared to the pointer?" - it is better than a pointer because it's an array, not a pointer. Array contains elements, which are writable - good. A pointer can point to any data, but in this case it points to read-only elements, so not good. – anatolyg Jul 31 '16 at 12:19

Andrew Cheong · Answer 2 · 2016-07-31T11:32:20.357

0

You're strcpying into static memory space, i.e.

char *data="xyz";

That is, you created a string xyz that was stored in a reserved memory location at compile-time. You can't write into that memory location. You want to allocate new space, e.g. malloc.

edited Jul 31 '16 at 11:32

answered Jul 31 '16 at 11:30

Andrew Cheong

29,362
15
90
145

[_"When you use a string literal in gcc the value is placed in read-only memory and cannot be modified. Trying to modify it leads to undefined behaviour."_](http://stackoverflow.com/questions/3843446/static-c-string-allocation-question) – Andrew Cheong Jul 31 '16 at 11:33
In C you would replace by `malloc(255*sizeof(char))` or even `malloc(255)` once you know that char is always 1 byte. The `new char[255]` part shall be replaced with that. `malloc(int)` is declared in ``. – Paul Stelian Jul 31 '16 at 11:33
Can someone explain what was wrong with my answer? I realize I typed `new` at first but that wasn't even the relevant part of the answer. Am I using the wrong words or something? – Andrew Cheong Jul 31 '16 at 11:47
I guess the `new` part was the only problematic part. To people who are not familiar with C++, this syntax looks like an exceedingly ugly syntax error. Now that you removed it, people are just too lazy to take their downvote back. – anatolyg Jul 31 '16 at 12:34

score 0 · Answer 3 · answered Jul 31 '16 at 11:41

0

char *data="xyz"; defines a char pointer to "xyz" which is located in read-only area so you can't write to this place. What point you still persist?

Change it to char data[] = "xyz" ensures this literal is copied to stack - so it's modifiable.

In addition, getcmdAtIndex function is not safe. What about out of index position?

answered Jul 31 '16 at 11:41

Soner from The Ottoman Empire

18,731
3
79
101

vote down? I have flagged the comment and vote downer. If there is something wrong in the answer, I will quit all SO network. – Soner from The Ottoman Empire Jul 31 '16 at 11:50
Wow you are rude. I tried to say that I wasn't the one who down vote you. Seems like a school here. – Curnelious Jul 31 '16 at 11:55
He is, he just removed his rude comment. – Curnelious Jul 31 '16 at 12:01
1

Well, no offense, but it might be your English @Curnelious. The way you wrote it, it sounds more like, "I don't know why people [would] do [it this way]. Vote [this] down." I'm not trying to be rude, but I could have read your comment wrong too. Your extra "this" makes it sound like the sentence ends there. Anyway, how about we all remove our comments and move on. I upvoted this answer as snr is correct. – Andrew Cheong Jul 31 '16 at 12:03
I was surprised that `[]` puts the literal on the stack, but it's true: http://stackoverflow.com/questions/2589949/c-string-literals-where-do-they-go – Andrew Cheong Jul 31 '16 at 12:04

Passing pointer and copy value

3 Answers3