Re: commit (HEAD): xap_Strings.h/cpp

From: Tomas Frydrych (tomasfrydrych_at_yahoo.co.uk)
Date: Fri Apr 09 2004 - 11:59:35 EDT

  • Next message: Dom Lachowicz: "Re: commit (HEAD): xap_Strings.h/cpp"

    > Hi Tomas,
    >
    >
    > You saw the problem, but didn't see the right
    > solution. Having static vars inside of getValue() and
    > getValueUTF8() is nasty.

    I am open to suggestions, but there are several factors to consider.

    > If the menus need to keep string references around longer, then the
    > problem is in the menu code, not XAP_String.

    Possibly, but not necessarily. In ap_Menu_Functions.cpp, line 64, we
    have this code:

    const char * c = pss->getValueUTF8(AP_STRING_ID_...).utf8_str();

    Here the UT_UTF8String temporary variable gets destroyed immediately
    as this code is executed, so c is never valid (this might be MSVC
    specific, but is not unreasonable). A solution in _this_ code would
    be to have instead

    UT_UTF8String s = pss->getValueUTF8(AP_STRING_ID_...);
    c = s.utf8_str();

    This will fix this problem but in my view it is a much poorer
    solution than having static variable in getValueUTF8(). As long as
    getValueUTF8() returns a temporary variable, this will happen again
    and again -- the problem is not immediately obvious, and because the
    memory that was used by the temporary variable might not be
    immediately reused, the pointer might sometimes appear to work.

    Returning a temporary variable from any function is just asking for
    trouble, creating bugs that are hard to reproduce and therefore fix;
    it is really the code in xap_Strings that is broken. In the interest
    of stability, functions should return pointers or references to non-
    local variables, not temporary objects.

    Tomas



    This archive was generated by hypermail 2.1.4 : Fri Apr 09 2004 - 12:02:14 EDT