CODEWITHSUNDEEP
cmemoryvalgrind
mik mik
mik mik

Reputation: 139

I don't understand why I get this valgrind error

I got the following code:

/* main.c */

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

int main (){
  int i;
  char *msg = "This is a simple and small message";
  int len = strlen (msg);
  char *new_msg = (char *) malloc (len);
  for (i = 0; i < len; i++)
    new_msg[i] = 'A';
  printf ("%s\n", new_msg);
  free (new_msg);
  return 0;
}

I compiled it and then run it using valgrind with this command:

valgrind --leak-check=full --show-reachable=yes ./main

I got the this output:

==8286== Memcheck, a memory error detector
==8286== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==8286== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==8286== Command: ./main
==8286== 
==8286== Invalid read of size 1
==8286==    at 0x4C2C1B4: strlen (vg_replace_strmem.c:412)
==8286==    by 0x4EA09FB: puts (ioputs.c:36)
==8286==    by 0x400636: main (main.c:12)
==8286==  Address 0x51de062 is 0 bytes after a block of size 34 alloc'd
==8286==    at 0x4C28C20: malloc (vg_replace_malloc.c:296)
==8286==    by 0x400601: main (main.c:9)
==8286== 
AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
==8286== 
==8286== HEAP SUMMARY:
==8286==     in use at exit: 0 bytes in 0 blocks
==8286==   total heap usage: 1 allocs, 1 frees, 34 bytes allocated
==8286== 
==8286== All heap blocks were freed -- no leaks are possible
==8286== 
==8286== For counts of detected and suppressed errors, rerun with: -v
==8286== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

I see that all the allocated memory was released, but I still get an error that I don't understand.

Appreciate the help.

Upvotes: 3

Views: 2088

Answers (3)

Haris
Haris

Reputation: 12270

There are a number of things to be changes in your code.

1) len should be of size_t not int, as strlen() returns of type size_t

2) (char *) malloc (len); drop the cast. This isn't an error, although there are reasons one should not cast.

3) new_msg is not NULL terminated \0. This is the reason the error is occurring.

Upvotes: 1

He Yuntao
He Yuntao

Reputation: 96

you use strlen() to get length, but not contain the '\0'.
so when you malloc a new array, you should use len + 1, and set new_msg[len] is '\0'.

Upvotes: 0

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726479

This is a pretty straightforward error: there is an invalid read of new_msg because the null terminator is not there.

You have allocated the number of chars equal to the length of the original string, so currently there's no space to fit '\0' without making undefined behavior. Change your code as follows to fix the problem:

char *new_msg = malloc (len+1);
for (i = 0; i < len; i++)
    new_msg[i] = 'A';
new_msg[len] = '\0';

Upvotes: 10

Related Questions