Memory leak through new[] without ever calling new

0
votes

I'm getting a memory leak from the following function:

int ReadWrite(int socket, char *readfile) {
  FILE *rf = NULL;
  rf = fopen(readfile, "rb");
  fseek(rf, 0, SEEK_END);
  int len = ftell(rf);
  rewind(rf);

  char readbuf[len + 1];

  int res = fread(readbuf, len, 1, rf);
  readbuf[len + 1] = '\0';
  fclose(rf);
  while (1) {
    int wres = write(socket, readbuf, res);
    if (wres == 0) {
      cerr << "socket closed prematurely" << endl;
      close(socket);
      return EXIT_FAILURE;
    }
    if (res == -1) {
      if (errno == EINTR)
        continue;
      cerr << "socket write failure: " << strerror(errno) << endl;
      close(socket);
      return EXIT_FAILURE;
    }
    break;
  }
  return EXIT_SUCCESS;
}

Valgrind tells me I leak the number of bytes that are in readfile (the actual file, not the name of readfile) through this operation:

Address 0x4c3b67e is 0 bytes after a block of size 14 alloc'd
at 0x4A07C84: operator new[](unsigned long) (vg_replace_malloc.c:363)

What's confusing me is I don't ever use new[] in my code. I checked fopen, ftell, and fread to see if they have hidden "gotcha's" where they call new[] somewhere but didn't find anything in the documentation on cplusplus.com. I've tried all different combinations of new char[]/delete[], malloc/free, and stack-allocated variables (above) but I get the same valgrind message every time. Any ideas? Thanks.

3
c++memory-leaksnew-operatorvalgrindfread
char readbuf[len + 1]; is invalid C++. – Luchian Grigore
This is a very strange code: 'char readbuf[len + 1]'. What compiler do you use? – Evgeny Eltishev
You should probably change readbuf[len + 1] = '\0'; to readbuf[len] = '\0';, unless you are purposely trying to overrun readbuf. Also, are you really using C99, rather than C++? – Markku K.

3 Answers

7
votes

you call

  • char readbuf[len + 1];

    and then later

  • readbuf[len + 1] = '\0';

wouldn't that overflow the array?

0
votes

Well, you are declaring your readbuf array with non-constant size (i.e with run-time size). This is formally illegal in C++. Such feature exists in C99, but not in C++. Your code will not even compile in a pedantic C++ compiler. And your question is tagged [C++].

But it is quite possible that your compiler implements this feature as a non-standard extension, and that it creates such arrays through an implicit call to new[]. This is why you get the error message that refers to new[], even though you are not using new[] explicitly.

Of course, it is the compiler's responsibility to deallocate such arrays when they end their lifetime. I suspect that the compiler does all it has to do, but the valgrind is confused by something in compiler's actions, which makes it conclude that it is a memory leak.

Moreover, as others already noted, you are making out of bounds access to your array, which can also lead to absolutely any problems at run-time, including the strange report from valgrind.

0
votes

I found out the problem actually had to do with the Makefile I was using. Thanks for the insight on my slip-up with the char[] bounds though!