5

I'm using gcc 8.2.1 and trying to build this code:

std::string dir = "Documents";
char * _tempname = static_cast <char*> (malloc( dir.length() + 14));
strncpy (_tempname, dir.c_str(), dir.length()+1 );
strncat (_tempname, "/hellooXXXXXX", 13);

but it gives me this warning:

warning: 'char* strncat(char*, const char*, size_t)' specified bound 13 equals source length [-Wstringop-overflow=]

After searching I found that it's an overflow problem to have the size_t equals the source length according to the discussion in this link but I couldn't understand why this is considered a problem and why this overflows the destination. And how could I remove this warning without changing my code?

Harshith Rai
  • 3,018
  • 7
  • 22
  • 35
Amr Essam
  • 96
  • 1
  • 1
  • 7
  • 2
    `strncat` expects a C-string as a first argument, but you're passing some uninitialized char array to it. Also why you're allocating memory manually when you could simply do `std::string tempname = dir + "/hellooXXXXXX"`? – HolyBlackCat Nov 21 '18 at 09:12
  • I forgot a line of code and I edited it now sorry, regarding the code why I'm not using string, actually I'm maintaining this code not writing it from scratch. – Amr Essam Nov 21 '18 at 09:16

3 Answers3

14

Apparently GCC understands that strncat(_tempname, "/hellooXXXXXX", 13); is no different from strcat(_tempname, "/hellooXXXXXX");, and finds it suspicious that you're using former instead of the latter.

If you can change the code, use strcat instead (or even better, rewrite the thing to use std::string).

If you can't change the code, use -Wno-stringop-overflow flag to disable the warning.

HolyBlackCat
  • 78,603
  • 9
  • 131
  • 207
3

my understanding is that gcc issues this warning only because it's a common mistake for users to set the bound equal to the src length, nothing more.

Yomna Kahwa
  • 31
  • 1
1

The function expects the space left in the destination not the length of the source string, so use

// ...
strncat(_tempname, "/hellooXXXXXX", dir.length() + 14 - strlen(_tempname) - 1);`

instead. No, forget that. Use std::string instead.

Swordfish
  • 12,971
  • 3
  • 21
  • 43