0

My C program reads a string from the terminal and makes a change to it like this:

//in int main
size_t n = 0;  char *inp = NULL;
while (getline (&inp, &n, stdin) != -1) {
   editStr("text", inp);
   fputs(inp, stdout);
   free(inp);
   n = 0;
   inp = NULL;
}



void editStr(char *add, char *inp){
  int newLength = strlen(inp) + strlen(add);
  char *p=malloc(newLength + 1);
  int addIndex = 0;
  for(int i = 0; i < newLength; i++){
    if(addIndex < strlen(add){
       p[i]=add[i];
       addIndex++;
    }
    else{
      p[i]=inp[i-addIndex];
    }
  }
  p[newLength] = '\0';
  inp = realloc(inp, newLength+1);
  strcpy(inp, p);
  free(p);
}

After running this on something like 

echo aabaac | valgrind -q --track-origins=yes ./program

This works correctly, but valgrind outputs a ton of errors. 

==6749== Invalid read of size 1
==6749==    at 0x483BBE6: strlen (vg_replace_strmem.c:461)
==6749==    by 0x48F7CF8: fputs (iofputs.c:33)
==6749==    by 0x40133F: main (program.c:72)

What have I done incorrectly with memory? I freed p correctly and line has been updated.

shurup
  • 751
  • 10
  • 33
  • 1
    You do not pass the return value of `realloc()` to the caller of editStr/editLine. You have to return the new pointer or use a double-pointer. – Ctx Feb 19 '20 at 22:49
  • 1
    `editStr` reassigns its local `inp` variable, but not the value of `inp` in `main()`. – Barmar Feb 19 '20 at 22:49
  • Didn’t `valgrind` tell you where the memory was released by `realloc()`? Your string copying code is complex and slow. Even if the compiler optimizes the nested call to `strlen()` out. Capture the length of the original string and the extra in separate variables. Use them in the `malloc()` call. Then use them in two calls to `memmove()` (or two calls two `strcpy()`, the second call using the length of the extra string as the offset to copy to. The `realloc()` call is superfluous; `p` is the same length and has the same contents, so you’re doing an extraneous copy. – Jonathan Leffler Feb 19 '20 at 23:52

0 Answers0