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

Was it helpful?

Solution

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.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top