Re: serhat - r31414 - in abiword/branches/gsoc2012docx/plugins/openxml: common/xp exp/xp imp/xp

On 24/06/12 09:24 AM, cvs@abisource.com wrote:
>
>
> Modified: abiword/branches/gsoc2012docx/plugins/openxml/exp/xp/ie_exp_OpenXML.cpp
> ===================================================================
> --- abiword/branches/gsoc2012docx/plugins/openxml/exp/xp/ie_exp_OpenXML.cpp 2012-06-23 05:10:59 UTC (rev 31413)
> +++ abiword/branches/gsoc2012docx/plugins/openxml/exp/xp/ie_exp_OpenXML.cpp 2012-06-24 16:24:11 UTC (rev 31414)
> @@ -24,6 +24,9 @@
> // Class definition include
> #include <ie_exp_OpenXML.h>
>
> +// Abiword includes
> +#include <ut_std_string.h>
> +
> /**
> * Constructor
> */
> @@ -2838,36 +2841,38 @@
> return UT_OK;
> }
>
> -UT_Error IE_Exp_OpenXML::startStyle(std::string style, std::string basedon, std::string followedby)
> +UT_Error IE_Exp_OpenXML::startStyle(std::string style, std::string basedon, std::string followedby, std::string type)
> {
> - UT_UTF8String sEscStyle = style.c_str();
> - UT_UTF8String sEscBasedOn = basedon.c_str();
> - UT_UTF8String sEscFollowedBy = followedby.c_str();
> + std::string sEscStyle = UT_escapeXML(style.c_str());
> + std::string sEscBasedOn = UT_escapeXML(basedon.c_str());
> + std::string sEscFollowedBy = UT_escapeXML(followedby.c_str());
> + std::string sEscType = UT_escapeXML(type.c_str());

UT_escapeXML takes a const std::string &, so you should be calling
c_str() on them to pass them.
(I wish there was a way to force this right now)

>
> Modified: abiword/branches/gsoc2012docx/plugins/openxml/exp/xp/ie_exp_OpenXML.h
> ===================================================================
> --- abiword/branches/gsoc2012docx/plugins/openxml/exp/xp/ie_exp_OpenXML.h 2012-06-23 05:10:59 UTC (rev 31413)
> +++ abiword/branches/gsoc2012docx/plugins/openxml/exp/xp/ie_exp_OpenXML.h 2012-06-24 16:24:11 UTC (rev 31414)
> @@ -86,7 +86,7 @@
> UT_Error finishParagraphProperties(int target);
> UT_Error startCellProperties(int target);
> UT_Error finishCellProperties(int target);
> - UT_Error startStyle(std::string name, std::string basedon, std::string followedby);
> + UT_Error startStyle(std::string name, std::string basedon, std::string followedby, std::string type);

I don't want to open a can of worm, but this is where you should also
change the signature to pass a const std::string & instead of a
std::string (that will be copy). Much more efficient.

> Modified: abiword/branches/gsoc2012docx/plugins/openxml/imp/xp/OXMLi_ListenerState_Common.cpp
> ===================================================================
> --- abiword/branches/gsoc2012docx/plugins/openxml/imp/xp/OXMLi_ListenerState_Common.cpp 2012-06-23 05:10:59 UTC (rev 31413)
> +++ abiword/branches/gsoc2012docx/plugins/openxml/imp/xp/OXMLi_ListenerState_Common.cpp 2012-06-24 16:24:11 UTC (rev 31414)

[...]

> + if(isOverline != std::string::npos && isUnderline == std::string::npos)
> + {
> + // if starts with "EQ \x \to", then overline property is used
> + UT_return_if_fail( this->_error_if_fail( UT_OK == run->setProperty("text-decoration", "overline") ));
> + rqst->stck->push(sharedElem);
> + }
> + else if(isUnderline != std::string::npos && isOverline == std::string::npos)
> + {
> + // if starts with "EQ \x \bo", then underline property is used
> + UT_return_if_fail( this->_error_if_fail( UT_OK == run->setProperty("text-decoration", "underline") ));
> + rqst->stck->push(sharedElem);
> + }

I know it is done elsewhere, but calling this kind of mutation function
like setProperty inside UT_return_if_fail() is not a good idea as I am
quasi certain that UT_return_if_fail() is what is causing some of our
file corruption bugs by exiting early because it is being abused.

Do the action then check the error.

Hub
Received on Thu Jun 28 07:56:05 2012