why strcpy doesn't work properly

Question

I want to copy each element of an array of strings to another array of strings, but when I copy one string whole array is changed to that string. Following is the code set.

This is the line of code having issue:

strcpy(MsgList[i].ga_data, a_Database[i].ga_data);

when I check the contents of a_Database[i].ga_data, it is as below

"1240,message 7:War of the worlds"

"1238,message 5:Life of this world"

"1236,message 3:world is not enough"

"1235,message 2:What a world!"

So instead of populating the same in MsgList[i].ga_data, it turns out to be just "1235,message 2:What a world!" 4 times, which is the last element copied.

typedef enum
{
  ACK,
  NACK,
  DELETE
}eMsgStatus_t;

typedef struct _Message
{
  eMsgStatus_t status;
  char *ga_data;
  uint16_t time;
}Message_t;
Message_t MessageList[8]={
  {ACK,"1234,message 1:Hello world",1000},
  {NACK,"1235,message 2:What a world!",1011},
  {NACK,"1236,message 3:world is not enough",1022},
  {ACK,"1237,messsge 4:Cruel world",1033},
  {NACK,"1238,message 5:Life of this world",1044},
  {ACK,"1239,message 6:Around the world in 80 days",1055},
  {NACK,"1240,message 7:War of the worlds",1066},
  {ACK,"1241,message 8:End of World",1077}
}; 

Message_t a_Database[20];
Message_t MsgList[20]= {0};

int main()
{
  for (i = 0; i < idx /* total unread message */; ++i)
  {
      strcpy(MsgList[i].ga_data, a_Database[i].ga_data);
      MsgList[i].ga_data[14] = '\0';
  }
}

uint8_t  GetMessages (Message_t *pg_Message)
{
  char i;uint8_t idx = 0;
   for (i = 0; i < 8 /* total message */; ++i)
  {
    if(NACK == MessageList[i].status)
    {
      pg_Message[idx].status = MessageList[i].status;
      pg_Message[idx].ga_data = MessageList[i].ga_data;
      pg_Message[idx].time = MessageList[i].time;
      idx++;
    }
  }
  return idx;
}

Your statement that the same string is copied to 4 elements and your example output disagree. Have you tried using a debugger to step through what is happening? If not, please learn this essential skill, which will help you understand your code, and save your and others' time by letting you solve your own problems instead of asking the internet. — underscore_d, Apr 18 '18 at 14:15
`strcpy(MsgList[i].ga_data, a_Database[i].ga_data);` target pointer isn't initialized. — Jean-François Fabre, Apr 18 '18 at 14:16
Hint: What is the value of `MsgList[i].ga_data` when `strcpy(MsgList[i].ga_data, a_Database[i].ga_data);` called? It is a pointer, but to what? — chux - Reinstate Monica, Apr 18 '18 at 14:16
@Jean-FrançoisFabre `MsgList[i].ga_data` is initialized (`0`) as it is a global - yet not to valid memory. — chux - Reinstate Monica, Apr 18 '18 at 14:17
I believe the downvotes are because the OP is blaming the function rather than their code? — Eugene Sh., Apr 18 '18 at 14:19
strcpying to MsgList[i].ga_data looks pretty bad to me, pointer is null — SCO, Apr 18 '18 at 14:27
Providing code is nice, but correct code is nicer. `idx` and `i` are not declared in `main`. — Pascal Cuoq, Apr 18 '18 at 14:38

score 1 · Answer 1 · answered Apr 18 '18 at 14:55

Well first of all you are corrupting memory in here:

strcpy(MsgList[i].ga_data, a_Database[i].ga_data);

since I cannot see where allocation for the MsgList[i].ga_data was done, I assuming it wasn't.

Then making null terminated string by magic number '14' here:

 MsgList[i].ga_data[14] = '\0';

Can lead to some memory corruption as well.

and even assuming that you making allocations somewhere, its very odd that you can observe full string in your MsgList[i].ga_data, since you making null termination at 14 it should be something like:

1235,message 2                                                            
1236,message 3
1238,message 5 
1240,message 7

why strcpy doesn't work properly

1 Answers1