0

I have the following code:

                ResultItemAttributeClass theAttribute;
                char* tmpObtainedValueName = (char*)obtainedValue.Name();
                theAttribute.AttributeName = (char*)malloc(strlen(tmpObtainedValueName)*sizeof(char));
                strcpy(theAttribute.AttributeName,tmpObtainedValueName);
                string tmpObtainedValueType0 = obtainedValue.aDesc->TypeName();
                char* tmpObtainedValueType = (char*) tmpObtainedValueType0.c_str();
                theAttribute.AttributeType = (char*)malloc(strlen(tmpObtainedValueType) * sizeof(char));
                strcpy(theAttribute.AttributeType, tmpObtainedValueType);
                string tmpAttributeValue0= obtainedValue.asStr();
                char* tmpAttributeValue = (char*) tmpAttributeValue0.c_str();
                theAttribute.AttributeValue = (char*)malloc(strlen(tmpAttributeValue) * sizeof(char));
                strcpy(theAttribute.AttributeValue, tmpAttributeValue);

ResultItemAttributeClass is declared as:

struct DLLDIR ResultItemAttributeClass {
    public:
      char* AttributeName;
      char* AttributeType;
      char* AttributeValue;
};

DLLDIR is a macro for export to DLL, yes, I am creating DLL , and std::string has questionable marshalling. In the code above I am filling instance of this structure, and storing it in some array, which is contained inside another structure (it's complex, I know) But when I try to call free on it:

free(inPntr->allItems[i].allAttributes[j].AttributeName);

I get a HEAP CORRUPTION messsage:

HEAP CORRUPTION DETECTED: after Normal block (#2606781) at 0x0000021EBB4FD060.
CRT detected that the application wrote to memory after end of heap buffer.

I have looked at similar questions about HEAP CORRUPTION :

I tried accessing AttributeName , AttributeValue, AttributeType before calling Free, and it works.

LOG_F(INFO, "| %s %s %s", inPntr->allItems[i].allAttributes[j].AttributeName, inPntr->allItems[i].allAttributes[j].AttributeType, inPntr->allItems[i].allAttributes[j].AttributeValue);

So, how do I keep the pointer to char* from std::string , so it does not get mingled and calling free on it does not cause crash . . . ?

Ivan P.
  • 832
  • 2
  • 9
  • 26
  • 1
    "author writes char beyond array boundary, but I am not modifying the value" - You *are* writing past the end of the array. You allocate `strlen(tmpObtainedValueName)` bytes, which does not include extra byte for null terminator, but then you use `strcpy`, which copies everything, including null terminator. – Yksisarvinen Aug 20 '21 at 09:59
  • 1
    `malloc(strlen(tmpObtainedValueName)*sizeof(char))` -> `malloc(strlen(tmpObtainedValueName)*sizeof(char)) + 1`. There must be 100s of duplicates – Jabberwocky Aug 20 '21 at 09:59
  • Might also be an issue: `char* tmpObtainedValueType = (char*) tmpObtainedValueType0.c_str()`: `tmpObtainedValueType` will become invalid when `tmpObtainedValueType0` goes out of scope. – Jabberwocky Aug 20 '21 at 10:02
  • 1
    Beyond all of those broken sizes, I'm truly curious what things like `(char*)obtainedValue.Name();` is actually accomplishing. What *exactly* was the reason you put that C-ctyle case there? What does the `Name` member of `obtainedValue` actually return ? Fyi, in nearly all general cases, and *especially* if you're fairly new to the language, C-style casts are a putrid code-smell that you're squelching warnings/errors into silence without actually *fixing* the real problem(s). – WhozCraig Aug 20 '21 at 10:02
  • So many great comments! Thank you all! @WhozCraig, a declaration of obtainedValue.Name() is: `const char *Name() const`. C++ thinks that const char* and char* are incompatible. @Jabberwocky - exactly, that's why I am moving content of tmpObtainedValueType using strcpy, and Visual Studio gives the warning too. – Ivan P. Aug 20 '21 at 10:28
  • @IvanP. That speaks to exactly what I'm referring to. There is no reason for `tmpObtainedValueName` to be `char*`. I can/should be `const char *`, thereby removing the unnecessary and potentially dangerous cast should the implementation and return type of `Name` ever change. Similarly, true for any/all `c_str()` results you're hard-casting to `char*`. It isn't needed. `strcpy`, takes `const char*` for the pointer you're using as the copy-source. `strlen` takes `const char*` for the single argument to find length. If something doesn't work without a hard-cast, casting is *rarely* the solution. – WhozCraig Aug 20 '21 at 10:30
  • The "questionable marshalling" is why Windows has the `BSTR` type, and MSVC has the `bstr_t` support class. – MSalters Aug 20 '21 at 10:35
  • @MSalters not that kind of marshalling. The ABI for a DLL exported object class/struct that exposes `std` artifacts as member variables will be hard-flagged by C4251 because none of the `std` library artifacts are similarly marked exported. See Ben's short, but to-the-point description [here](https://stackoverflow.com/a/33965273/1322972). – WhozCraig Aug 20 '21 at 10:38

0 Answers0