1

Take the example code*:

char *string = (char*)malloc(sizeof(char));
strcat_s(string, strlen(string) + 10 + 1, "characters");

The above code compiles and runs, leading me to believe that memory reallocation is taking place. However when applied on a greater scale (recursively also), I receive memory errors in random places (different each time the program is run).

Could strcat_s() be exceeding boundaries? Is realloc() therefore required to ensure the memory is properly allocated?

Note: It could be that the errors are unrelated, although they have been coincidentally cropping up after applying the code from the example.

*The reason I've only allocated one byte initially, is that contextually I'm working with dynamic sizes, so the size of string will change, but by an unknown amount.

πάντα ῥεῖ
  • 1
  • 13
  • 116
  • 190
  • 1
    Check the [reference](http://en.cppreference.com/w/cpp/string/byte/strcat): *The behavior is undefined if the destination array is not large enough for the contents of both src and dest and the terminating null character.* – NathanOliver Mar 08 '17 at 15:57
  • 1
    I wouldn't recommend using `malloc`, `strlen`, or `strcat_s` in C++. There are better tools. – Fred Larson Mar 08 '17 at 15:58
  • @FredLarson Quite so - I have considered using std::string, but it sort of feels like cheating. But given the arduous nature of manual memory management on a larger scale, I think I might have to. – Synthetic Ascension Mar 08 '17 at 16:00
  • It's not cheating to use the right tools for the job. – Fred Larson Mar 08 '17 at 16:04
  • 1
    " leading me to believe that memory reallocation is taking place" - none of the standard str*** functions allocate memory. –  Mar 08 '17 at 16:04
  • @SyntheticAscension: Why "cheating"? If I could use nothing from C++ other than string and vector, that alone would be worth the switch. If you're into "actual" string processing, though (the stuff including lowercase / uppercase conversions, regexes etc.), I'd take a look at [icu::UnicodeString](http://site.icu-project.org/). A bit clumsier to handle (as it's based off the Java code base, lacking some of the scaffolding you'd expect from a C++ class), but basically the only way to get "real" (Unicode) string handling capabilities. – DevSolar Mar 08 '17 at 16:05
  • Maybe you should give us a broader picture of what you are _actually_ trying to achieve. See also [XY Problem](http://xyproblem.info/). – Jabberwocky Mar 08 '17 at 16:13
  • @FredLarson The question isn't tagged C++ so `std::string` is out. – JeremyP Mar 08 '17 at 16:34
  • @JeremyP: It was when I commented. – Fred Larson Mar 08 '17 at 16:36
  • @FredLarson Oh yes. In fact, it was **only** tagged C++, in which case "use `std::string`" would have been the best answer. – JeremyP Mar 08 '17 at 16:40

2 Answers2

2

The above code compiles and runs, leading me to believe that memory reallocation is taking place.

Just because the program appears to behave as you expect does not mean that it is correct, or even that its behavior is defined at all from C's perspective.

However when applied on a greater scale (recursively also), I receive memory errors in random places (different each time the program is run).

Could strcat_s() be exceeding boundaries?

Yes.

Is realloc() therefore required to ensure the memory is properly allocated?

No.

Neither strcat_s() nor strcat() performs any reallocation. They are not specified to do so, and it would be unsafe for them to do so.

You receive errors because you are using the function incorrectly (even when you don't get errors). It is your responsibility to ensure that the second argument does not exceed the size of the array pointed to by the first, but you are flagrantly disregarding that responsibility. I presume you simply have a serious misunderstanding about what strcat_s() is supposed to do, and what its second parameter means.

The main thing that strcat_s() provides but strcat() doesn't is checking that the specified array bounds are not overrun as a result of the second string being longer than can be accommodated. This relieves you of checking the length of the second string before performing the concatentation, which is advantageous because strcat_s() can do that itself at very low cost, since it must scan that string anyway. strcat_s() has no more ability than any other C operation or function to determine independently how long is the array to which the first argument points. It relies on you to tell it.

If you need to accommodate dynamic adjustment of your array size, then that's your responsibility, as is tracking the current size of the allocation.

John Bollinger
  • 160,171
  • 8
  • 81
  • 157
1

Here you allocate exactly 1 char

 char *string = (char*)malloc(sizeof(char));

so the only string the string can hold is "" (the zero length string)

Then you try to append the string "characters" to string which cannot hold a string other than "" and which is not initialized. Furthermore the result of strlen(string) will be undetermined, because again string is not initialized.

 strcat_s(string, strlen(string) + 10 + 1, "characters");

You probably want this:

char *string = (char*)malloc(sizeof(char) * 100);   // allocate 100 bytes
strcpy(string, "Hello ");
strcat_s(string, 100, "characters");

printf("%s\n", string);  // will display "Hello characters".
Jabberwocky
  • 48,281
  • 17
  • 65
  • 115
  • 1
    I always like to point out that `sizeof(char)` is 1 by definition, so you could just as well use `malloc(100)`. – Fred Larson Mar 08 '17 at 16:19
  • @FredLarson entirely correct, but if I had written `malloc(100)`, someone else would certainly comment _I'd rather write `sizeof(char) * 100` to emphasize we're allocating chars_. – Jabberwocky Mar 08 '17 at 16:23
  • I'd be one of them :) Prefer `sizeof(char) * 100` to just `100`. Actually, I'd prefer `calloc(100, sizeof(char))`. – JeremyP Mar 08 '17 at 16:31