Memory corruption warning workaround

[wxBen]

I have been using the wxHtmlWindow and it works very nicely. However, with all possible debug and memory corruption and heap checks turned on, Visual Studio stops on the following lines and complains about heap corruption and you cannot continue on:

wxSscanf(wd.c_str(), wxT("%i"), &width);

Now I have been debugging this, and it is a surprisingly complex line, with lots of temporary objects with buffers and ref counts being created for example. Even though the heap does indeed seem to get corrupted, despite careful debugging and being in the one in our group who fixes memory corruption issues, I could not find any real cause for it.

However, if you look at other functions in the files in question, in several places it uses this:

                    tag.GetParamAsInt(wxT("HEIGHT"), &h);

with GetParamAsInt really just using wxString::ToLong and casting to an int with the return value being ignored really.

Likely that is not used in these cases as the param string value has already been extracted, and just needs to be converted to an int. But I think the cost of wxSscanf is considerably more than GetParamAsInt which is still more than you need here as well. The string is essentially just one number, nothing complex or multiple arguments being extracted or anything.

So the solution to this issue seems to be to simplify the code. So I replaced the lines in question with simpler equivalents. That seems to work and Visual Studio is no longer complaining about memory corruption on those lines.

Please carefully check and verify.

[vadz]

I don't see any problems from the debug MSVC CRT, what exactly do I need to do to reproduce them? I'd be more interested in fixing the real problem in wxSscanf() than working around it in wxHTML code.

But I do agree that wxHTML could have a function to read a number which can be expressed either as an absolute value or a percentage from a string, this would be useful to have, something like this (warning: totally untested code follows):

bool GetParamAsAbsOrPercent(const wxString& param, int* value, wxHtmlUnits* units)
{
   wxString num;
   if ( param.EndsWith("%", &num) ) {
     *units = wxHTML_UNITS_PERCENT;
   }
   else {
     num = param;
     *units = wxHTML_UNITS_PIXELS;
   }

   long lValue;
   if ( !num.ToLong(&lValue) )
     return false;

   if ( lValue > INT_MAX || lValue < INT_MIN )
     return false;

   *value = static_cast<int>(lValue);

   return true;
}

with wxHtmlUnits being an enum containing wxHTML_UNITS_PIXELS and wxHTML_UNITS_PERCENT.

[wxBen]

We compile with /Zp1 /RTC1 /J.

As I said, I have debugged this, but could not see the cause of the issue. But I have started a compile on our Purify machine and will run it on there to see if it finds anything. I shall advise on that front.

Please find attached a new patch, as per your suggestion, which does clean up the code a bit and also avoids this issue (and thus makes me happy).

[wxBen]

HTMLVS2010.diff

[VZ]

(In [73142]) Add a wxHtmlTag helper parsing both absolute values and percents.

This allows to avoid some code duplication in different handlers.

Closes #14868.

[wxBen]

Vadim, for your information, if I run this under Purify, then the error does not come up, and it merely warns about uninitialized memory. File is attached. Have a look at the first three items in the file, they are likely false alarms? My own careful repeated debugging efforts also could not find any memory corruption. So I guess this is a workaround / cleanup that avoids an issue on VC++.

The fourth one in the file about m_help_button_hovered being unintiailized in ribbon\bar.cpp appears to be indeed a real issue, and needs fixing in the constructor.

[wxBen]

Purify.txt

[vadz]

I wonder if these UMRs couldn't be due to the small string optimization in the CRT. I.e. perhaps it really doesn't initialize the unused parts of string representation (it uses either the fixed size buffer or the dynamically allocated pointer) but still swaps both of them. In which case the warning is correct but harmless...

I'll fix the one about the really uninitialized variable, thanks.

[VZ]

(In [73161]) Initialize wxRibbonBar::m_help_button_hovered.

This variable was never initialized, do it now.

See #14868.