Having problems with pointers in c

Question

I have structure

typedef struct song_
{
     char  *artist ;
     char  *title ;
     mtime *lastPlayed ;
} song ;

and a function that takes a pointer to a song struct and returns a pointer to a copy of that song

song *songCopy(const song *s){
  song *d = NULL ;
  mtime *tmp = NULL ;

  d = malloc(sizeof(song)) ;

  d->artist = malloc(sizeof(*s->artist) + 1) ;
  strcpy(d->artist, s->artist) ; //****

  d->title = malloc(sizeof(*s->title) + 1) ;
  strcpy(d->title, s->title) ;  //****

  if (NULL != s->lastPlayed)
    {
      // copy the last played
      tmp = mtimeCopy(s->lastPlayed) ;
      d->lastPlayed = tmp ;
    }
  else
    {
      // set lastPlayed to NULL
      d->lastPlayed = NULL ;
    }

  return d ;
  }

I'm debugging this with valgrind and I'm getting this error message on the starred lines above

Invalid write of size 1
==14096==    at 0x402C6C3: strcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==14096==    by 0x8048EC9: songCopy (song.c:104)
==14096==    by 0x80487C6: main (songtest.c:82)
==14096==  Address 0x41fe7a2 is 0 bytes after a block of size 2 alloc'd
==14096==    at 0x402BE68: malloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==14096==    by 0x8048EAA: songCopy (song.c:103)
==14096==    by 0x80487C6: main (songtest.c:82)

I feel like I have change something with how d is declared but I don't know what

As far as I can tell you need to use `strlen` not `sizeof` here `sizeof(*s->artist)` and here `sizeof(*s->title) + 1`. — Shafik Yaghmour, Feb 23 '14 at 01:38
If you does not like this question, **delete the question itself**, please do not delete the contents of your question, this will make other readers of this question very confused. — Lee Duhem, Feb 23 '14 at 04:56
Also, deleting a perfectly good question like this (which was successfully answered) would be completely inconsiderate to Stack Overflow as a whole. I hope your edit was somehow a mistake. — Jonathon Reinhart, Feb 23 '14 at 05:47

score 1 · Accepted Answer · answered Feb 23 '14 at 01:38

1

The easist way would be to use strdup to duplicate the strings.

  d->artist = strdup(s->artist);
  d->title = strdup(s->title);

To fix your actual problem, you need to use strlen and not sizeof.

  d->artist = malloc(strlen(s->artist) + 1) ;
  strcpy(d->artist, s->artist);

  d->title = malloc(strlen(s->title) + 1) ;
  strcpy(d->title, s->title);

The problem is that this:

  sizeof(*s->artist)

Is returning sizeof(char), which is most likely 1.

answered Feb 23 '14 at 01:38

Jonathon Reinhart

132,704
33
254
328

for more on that topic, please see that [answer](http://stackoverflow.com/a/14021039/1290438) – zmo Feb 23 '14 at 01:40

Having problems with pointers in c

1 Answers1