Bug Hunting Session
Bug 112689 - Replace chained O(U)StringBuffer::append() with operator+
Summary: Replace chained O(U)StringBuffer::append() with operator+
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
6.0.0.0.alpha0+
Hardware: All All
: medium enhancement
Assignee: Not Assigned
URL:
Whiteboard: target:6.0.0 target:6.1.0 target:6.3.0
Keywords: difficultyBeginner, easyHack, skillCpp, topicCleanup
Depends on:
Blocks:
 
Reported: 2017-09-27 09:21 UTC by Muhammet Kara
Modified: 2019-04-08 07:22 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
I Replace chained O(U)StringBuffer::append() with operator+ (21.71 KB, patch)
2019-01-27 21:34 UTC, Yusuf Sönmez
Details
I Replace chained O(U)StringBuffer::append() with operator+ (64.11 KB, patch)
2019-01-28 23:14 UTC, Yusuf Sönmez
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Muhammet Kara 2017-09-27 09:21:25 UTC
This is a follow-up from https://bugs.documentfoundation.org/show_bug.cgi?id=57950

The codebase contains code such as:

OUStringBuffer buf;
buf.append( "pyuno bridge: expected for method " );
buf.append( aFunctionName );
buf.append( " one return value and " );
buf.append( aOutParamIndex.getLength() );
buf.append( " out parameters, got a sequence of " );
buf.append( seq.getLength() );
buf.append( " elements as return value." );

which could be more easily written as:

OUString sMsg = "pyuno bridge: expected for method "
              + aFunctionName
              + " one return value and "
              + OUString::number(aOutParamIndex.getLength())
              + " out parameters, got a sequence of "
              + OUString::number(seq.getLength())
              + " elements as return value.";

All these chained calls to OStringBuffer::append() and OUStringBuffer::append() can be simply replaced by usage of operator+ as written in the example above.

Here is an example commit: https://cgit.freedesktop.org/libreoffice/core/commit/?id=7810858ee6cac233ce5868de1f2ef5de1d443af4

(You may also check the related bug reports for more examples.)

And you may check these files for some possible instances:
pyuno/source/module/pyuno_module.cxx
extensions/source/config/ldap/ldapaccess.cxx
extensions/source/propctrlr/selectlabeldialog.cxx
starmath/source/dialog.cxx
starmath/source/rtfexport.cxx
filter/source/graphicfilter/ipict/shape.cxx
ucb/source/ucp/ftp/ftpurl.cxx
ucb/source/core/provprox.cxx
desktop/source/deployment/registry/component/dp_component.cxx
svtools/source/misc/transfer.cxx
svtools/source/svhtml/htmlout.cxx
vcl/source/uitest/uiobject.cxx
vcl/source/window/mnemonic.cxx
vcl/unx/generic/fontmanager/fontconfig.cxx
cui/source/options/certpath.cxx
cui/source/options/optgdlg.cxx
cui/source/options/optlingu.cxx
unotools/source/config/configpaths.cxx
unotools/source/config/bootstrap.cxx
sax/source/tools/converter.cxx
connectivity/source/commontools/sqlerror.cxx
connectivity/source/drivers/firebird/Util.cxx
idlc/source/options.cxx
sc/source/ui/dbgui/foptmgr.cxx
sc/source/ui/condformat/condformathelper.cxx
sc/source/ui/condformat/condformatmgr.cxx
sc/source/ui/vba/vbahyperlink.cxx
sc/source/ui/view/tabview.cxx
sc/source/ui/view/cellsh1.cxx
sc/source/ui/view/editsh.cxx
sc/source/filter/html/htmlexp2.cxx
sc/source/filter/html/htmlexp.cxx
sc/source/filter/dif/difimp.cxx
sc/source/filter/oox/querytablebuffer.cxx
sc/source/filter/oox/worksheethelper.cxx
sc/source/filter/oox/formulabase.cxx
sc/source/filter/oox/drawingfragment.cxx
sc/source/filter/oox/autofilterbuffer.cxx
sc/source/filter/excel/xipivot.cxx
sc/source/filter/excel/xehelper.cxx
sc/source/filter/excel/xiescher.cxx
sc/source/core/tool/compiler.cxx
sc/source/core/tool/address.cxx
sc/source/core/tool/chgtrack.cxx
sc/source/core/data/postit.cxx
sc/source/core/data/validat.cxx
sc/qa/unit/subsequent_filters-test.cxx
sc/qa/unit/helper/qahelper.cxx
sal/test/testbootstrap.cxx
xmlhelp/source/cxxhelp/provider/databases.cxx
sw/source/ui/config/mailconfigpage.cxx
sw/source/ui/vba/vbarow.cxx
sw/source/ui/vba/vbacolumn.cxx
sw/source/filter/html/htmltabw.cxx
sw/source/filter/html/htmlflywriter.cxx
sw/source/filter/html/htmlftn.cxx
sw/source/filter/html/swhtml.cxx
sw/source/filter/html/htmldrawwriter.cxx
sw/source/filter/html/htmlplug.cxx
sw/source/filter/html/wrthtml.cxx
sw/source/filter/html/htmlfldw.cxx
sw/source/filter/html/htmlbas.cxx
sw/source/filter/ww8/docxsdrexport.cxx
sw/source/filter/ww8/rtfsdrexport.cxx
sw/source/filter/ww8/docxattributeoutput.cxx
sw/source/filter/ww8/wrtw8esh.cxx
sw/source/filter/ww8/rtfattributeoutput.cxx
sw/source/filter/ww8/rtfexport.cxx
sw/source/uibase/dbui/dbmgr.cxx
sw/source/uibase/docvw/edtwin.cxx
sw/source/uibase/docvw/edtwin2.cxx
sw/source/uibase/ribbar/inputwin.cxx
sw/source/core/doc/docbm.cxx
sw/source/core/crsr/bookmrk.cxx
sw/qa/core/layout-test.cxx
sw/qa/core/macros-test.cxx
basic/source/runtime/methods.cxx
sfx2/source/dialog/versdlg.cxx
sfx2/source/appl/childwin.cxx
sfx2/source/appl/newhelp.cxx
sfx2/source/appl/appuno.cxx
sfx2/source/appl/macroloader.cxx
sfx2/source/bastyp/frmhtmlw.cxx
oox/source/vml/vmlinputstream.cxx
oox/source/vml/vmlformatting.cxx
oox/source/ole/vbamodule.cxx
oox/source/export/vmlexport.cxx
oox/source/core/xmlfilterbase.cxx
oox/source/dump/dumperbase.cxx
oox/source/drawingml/chart/objectformatter.cxx
oox/source/drawingml/chart/titlecontext.cxx
oox/source/drawingml/lineproperties.cxx
i18npool/source/collator/collator_unicode.cxx
i18npool/source/breakiterator/breakiteratorImpl.cxx
i18npool/source/breakiterator/xdictionary.cxx
i18npool/source/defaultnumberingprovider/defaultnumberingprovider.cxx
i18npool/source/localedata/localedata.cxx
sdext/source/pdfimport/tree/genericelements.cxx
workdir/UnpackedTarball/hunspell/src/tools/hunspell.cxx
workdir/UnpackedTarball/icu/source/test/perf/DateFmtPerf/DateFmtPerf.h
workdir/UnpackedTarball/icu/source/common/unicode/localpointer.h
workdir/UnpackedTarball/firebird/src/dsql/DdlNodes.h
workdir/UnpackedTarball/firebird/src/common/StatusArg.h
sd/source/ui/remotecontrol/Listener.cxx
sd/source/core/drawdoc3.cxx
sd/source/core/sdpage2.cxx
sd/source/core/sdpage.cxx
editeng/source/misc/svxacorr.cxx
xmloff/source/forms/elementimport.cxx
xmloff/source/forms/layerimport.cxx
svl/source/misc/urihelper.cxx
svl/source/numbers/zforlist.cxx
Comment 1 Commit Notification 2017-10-04 07:01:52 UTC
Arkadiy Illarionov committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=9db36aac932f688adc492a90a7cd4b350dd19ed8

tdf#112689 Replace chained O(U)StringBuffer::append with operator+ in cui

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 2 Muhammet Kara 2017-10-12 18:52:28 UTC
Please also note that this easyhack is not meant for O(U)StringBuffer's inside loops.
Comment 3 Commit Notification 2017-10-14 07:54:13 UTC
ekuiitr committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2b5b8fd39c93166d9e6e696c010482413b155f78

tdf#112689 - Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 4 Commit Notification 2017-10-14 21:56:54 UTC
ekuiitr committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8eacd3be08bf6e1a97900624611822de9b00a379

tdf#112689 - Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 5 Commit Notification 2017-10-18 04:53:58 UTC
ekuiitr committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=a4a182e24d2e3e954831a0a7c70a7299f28950cb

tdf#112689 - Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 6 Commit Notification 2017-10-30 10:29:03 UTC
Furkan Tokac committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=7939e29b4117733005f81960ca792b2629d63212

tdf#112689 - Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Commit Notification 2017-10-30 10:38:07 UTC
Furkan Ahmet Kara committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c7660cc543903c26c30d22e70e42deab9573c94d

tdf#112689 - Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2017-11-22 20:19:27 UTC
qzheng committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=1783b33b52c4332be6163d15ae78bbeb4e16c172

tdf#112689 Replace chained O(U)StringBuffer::append with operator+

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Stephan Bergmann 2017-11-23 08:29:26 UTC
(In reply to Muhammet Kara from comment #2)
> Please also note that this easyhack is not meant for O(U)StringBuffer's
> inside loops.

And also note that these changes are only really beneficial if the resulting OUString is created in one go,

  OUString s = ... + ... + ... + ...;

Replacing existing piecemeal OUStringBuffer construction with

  OUString s = ...;
  ...
  s += ...;
  ...
  s += ...;
  ...
  s += ...;

isn't that useful (and potentially has negative impacts on memory usage).
Comment 10 Commit Notification 2017-11-23 12:40:05 UTC
Timotej Lazar committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=6a97d9c4d9b80db41149740e4fb7706142d1b20c

tdf#112689 Replace chained O(U)StringBuffer::append with operator+

It will be available in 6.0.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Commit Notification 2018-02-22 12:58:24 UTC
Shubham Verma committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=0c371a6b5e6c6251d782c7a8ecbe0ab32c7812ac

tdf#112689 : Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.1.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 12 Muhammet Kara 2018-02-26 06:25:30 UTC
No need to assign this bug report to yourself. It is intended to be solved by many/different newcomers with small patches. Just send your patch on gerrit.
Comment 13 Noel Grandin 2018-07-30 11:03:12 UTC
A search like this:

   git grep -A1 -wn OUStringBuffer | grep -B1 -w append

will find these places
Comment 14 Commit Notification 2018-12-30 13:44:29 UTC
Çağrı Dolaz committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/6f967e3403c6675bbaf2c17dcadf7e640dd719ca%5E%21

tdf#112689: Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 15 Commit Notification 2019-01-10 07:51:31 UTC
Doğa Deniz Arıcı committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/f0f462bfa5465aa978c82e6c4aad058d9b760e93%5E%21

tdf#112689: Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 16 Commit Notification 2019-01-11 17:35:33 UTC
Furkan Ahmet Kara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/a47b8f223847ce28330244bc0eef2e6b72f7f989%5E%21

tdf#112689:Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Yusuf Sönmez 2019-01-27 21:34:25 UTC
Created attachment 148695 [details]
I Replace chained O(U)StringBuffer::append() with operator+

I Replace chained O(U)StringBuffer::append() with operator+ for;

https://cgit.freedesktop.org/libreoffice/core/tree/ucb/source/ucp/ftp/ftpurl.cxx

It's my first patch. I checked this patch a lot of, it may be have a wrongs,but it will continue in future.Thanks
Comment 18 Yusuf Sönmez 2019-01-28 23:14:25 UTC
Created attachment 148718 [details]
I Replace chained O(U)StringBuffer::append() with operator+

for; 
https://cgit.freedesktop.org/libreoffice/core/tree/desktop/source/deployment/registry/component/dp_component.cxx
Comment 19 Stephan Bergmann 2019-01-29 07:34:06 UTC
(In reply to Yusuf Sönmez from comment #18)
> Created attachment 148718 [details]
> I Replace chained O(U)StringBuffer::append() with operator+

please use the LibreOffice Gerrit instance to submit patches, see <https://wiki.documentfoundation.org/Development/GetInvolved#Prepare_to_submit_patches> for details
Comment 20 Xisco Faulí 2019-02-05 17:32:20 UTC
No need to assign this bug report to yourself. It is intended to be solved by many/different newcomers with small patches. Just send your patch on gerrit.
Comment 21 Commit Notification 2019-02-11 07:53:03 UTC
Meryem Ezber committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/2edaadce0dedbee5d4a8213ea06d04cfc3c6cf55%5E%21

tdf#112689: Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 22 Commit Notification 2019-02-11 08:10:22 UTC
Muzaffer Kadir YILMAZ committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/3e214676b95ac384ae938f7baf7a4edbd4f5a7f3%5E%21

tdf#112689: Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 23 Commit Notification 2019-02-11 08:17:26 UTC
Omer Fatih Celik committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/867384792244667a33cad79a7348b34b6008822b%5E%21

tdf#112689: Replace changed O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 24 Commit Notification 2019-02-11 12:06:12 UTC
Meryem Ezber committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/2dd7aba7564a222c2acbac22975a76a6ab33c41f%5E%21

tdf#112689: Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 25 Commit Notification 2019-02-11 13:19:08 UTC
Tor Lillqvist committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/4f7f1dd12406655f71195c577ba7f7d128a93ce4%5E%21

Revert "tdf#112689: Replace changed O(U)StringBuffer::append() with operator+"

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 26 Commit Notification 2019-02-11 13:34:02 UTC
Yusuf Sonmez committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/3b5dd1d49e3b5d669d020f0b268625b106c1c661%5E%21

tdf#112689: Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 27 Luke 2019-02-11 18:21:13 UTC
Before submitting a patch, you should run a 'make check' build with

--enable-64-bit

Jenkins alone is not enough as it only runs a single, minimal 32-bit build.
Comment 28 Commit Notification 2019-02-19 16:30:56 UTC
Salih Sariyar committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/4d6c45c80ae37e2b92f37543329799e8c60f6021%5E%21

tdf#112689:Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 29 Commit Notification 2019-02-19 16:39:07 UTC
Salih Sariyar committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/12180ed8d6d64f78d37c6ee070da5a1ab3684843%5E%21

tdf#112689:Replace chained O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 30 Commit Notification 2019-02-22 10:54:49 UTC
Omer Fatih Celik committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/c2fce89079f10103e64eaa7a687b131bc36cd8af%5E%21

tdf#112689: Replace changed O(U)StringBuffer::append() with operator+

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 31 Commit Notification 2019-04-08 07:22:00 UTC
Jason Burns committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/642bd339576537d6561735273bda64dd44858960%5E%21

tdf#112689: replace OUStringBuffer with OUString in one file

It will be available in 6.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.