Bug 140226 - Make use of SfxPoolItems more typesafe by using StaticWhichCast
Summary: Make use of SfxPoolItems more typesafe by using StaticWhichCast
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Writer (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:7.2.0 target:7.3.0 target:7.6.0
Keywords: difficultyMedium, easyHack, skillCpp, skillTest, topicCleanup
Depends on:
Blocks: Dev-related
  Show dependency treegraph
 
Reported: 2021-02-07 00:52 UTC by Björn Michaelsen
Modified: 2023-05-15 21:36 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Björn Michaelsen 2021-02-07 00:52:43 UTC
Description:
There is a lot of code of the form:

    switch ( rItem.Which() )
    {
        case RES_CHRATR_CASEMAP:
            rFnt.SetCaseMap( static_cast<const SvxCaseMapItem&>(rItem).GetCaseMap() );
            break;
    ...

in Writer and elsewhere. That is: code that uses SfxPoolItems home-grown "WhichId" as type information and then uses a static_cast to downcast the base class to a proper type. As this "WhichId" is generated at runtime, this is errorprone and fragile. Since the introduction of TypedWhichId in include/svl/typedwhich.hxx (Thanks Noel!), there exists a proper mapping of WhichIds to the derived classes of SfxPoolItem. SfxPoolItem::StaticWhichCast uses these to do a proper downcast, while asserting if either the WhichId or the dynamic type does not match expectations. This EasyHack is to:

1/ Find such a static_cast. "git grep static_cast.*Item" might be a starting
   point.
2/ Replace the static_cast with a StaticWhichCast. An example of how this is done
   can be found at: https://gerrit.libreoffice.org/c/core/+/110233
3/ Ensure the debug builds still pass all tests after the change (gerrit/jenkins
   can help here.)

What is to be done, if one of the StaticWhichCasts asserts? Congratulations, you likely just dicovered some type confusion in LibreOffice. You now have three options to resolve this issue:
a/ Investigate how the misleading item ended up in this place and fix it. This
   might be non-trivial.
b/ Put a big, fat FIXME note in the source code hinting that the content of the
   SfxPoolItem defies expectations, use a DynamicWhichCast instead and guard the
   following code against the case that the cast produces a nullptr.
c/ Leave this particular static_cast as is and ad a big, fat FIXME not as per
   above.

Ultimately, a/ is the best option here, but it might be out of the scope of an EasyHack. If you have help available (e.g. from #libreoffice-dev IRC), you might still venture to solve it.

Steps to Reproduce:
.

Actual Results:
.

Expected Results:
.


Reproducible: Always


User Profile Reset: No



Additional Info:
.
Comment 1 Commit Notification 2021-02-23 16:21:58 UTC
Bjoern Michaelsen committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/a3f83ecbeab33244b526ec68ca0f2034745a6371

tdf#140226: use StaticWhichCast

It will be available in 7.2.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 2 Commit Notification 2021-04-09 08:50:24 UTC
Ahmet Hakan Çelik committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/7f9a66cb11f0889bf10542a17c144f9665c0ce70

tdf#140226: Make use of SfxPoolItems more typesafe by using StaticWhichCast

It will be available in 7.2.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 3 Commit Notification 2021-08-14 06:36:24 UTC
Emircan Agac committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/da76d817400242934fdaebcdb226abf3d6595f56

tdf#140226: use StaticWhichCast

It will be available in 7.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 4 Commit Notification 2021-08-14 06:37:35 UTC
Emircan Agac committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/1fdb3f98590532705be9972d031eb09027795760

tdf#140226: use StaticWhichCast

It will be available in 7.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 5 Commit Notification 2021-08-14 06:37:44 UTC
Emircan Agac committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/d5eceef8e02886edc142267ee8d74f7adb198a1b

tdf#140226: use StaticWhichCast

It will be available in 7.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 6 Commit Notification 2021-09-17 08:52:58 UTC
Baran Aytas committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6184eefa835f0495ed8136471d61837f5662b6d6

tdf#140226: Make use of SfxPoolItems more typesafe by using StaticWhichCast

It will be available in 7.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 7 Commit Notification 2023-03-16 09:50:47 UTC
Bayram Çiçek committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/66d38ba8e9ccb7ba0ef5b3feb67b241a5b6b2e80

tdf#140226: use StaticWhichCast in htmlatr.cxx

It will be available in 7.6.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 8 Commit Notification 2023-05-15 21:36:21 UTC
MoazAlaa committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/a9d60aba81305bf86024df707eb694d1cabc9a1d

tdf#140226: use StaticWhichCast

It will be available in 7.6.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.