[Libreoffice-commits] core.git: sal/rtl

Stephan Bergmann (via logerrit) logerrit at kemper.freedesktop.org
Fri Apr 23 17:06:46 UTC 2021


 sal/rtl/string.cxx  |    7 +++++++
 sal/rtl/ustring.cxx |    7 +++++++
 2 files changed, 14 insertions(+)

New commits:
commit 8ae3ae4bf75fdd0aaa132c956d9da029baa3adc6
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Fri Apr 23 13:28:37 2021 +0200
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Fri Apr 23 19:06:05 2021 +0200

    Step 1 of removing cargo-cult pragma pack around rtl_[u]String
    
    Following up on f62cb40bdfaf41cf8e989640f9be79f652f30914 "Remove dubious #pragma
    pack" and 9eba8aa38db3a0dc2f7dfaf24a003c16418aef18 "Remove dubious #pragma pack"
    for O[U]StringLiteral, which argued that the pragma pack around rtl_[u]String
    are useless cargo cult (paraphrasing):  "All struct member types involved
    (oslInterlockedCount aka sal_Int32, sal_Int32 itself, sal_Unicode, and char)
    have size <= 4 resp. < 8, so the member alignment ("on a boundary that's either
    a multiple of [the pragma pack value], or a multiple of the size of the member,
    whichever is smaller", according to
    <https://docs.microsoft.com/en-us/cpp/preprocessor/pack?view=msvc-160>) is not
    affected.  And neither are alignof(rtl_String) and alignof(rtl_uString)
    affected, which would remain e.g. 4 on x86-64."
    
    (Curiously, the pragma pack value had always been 8 for rtl_String but 4 for
    rtl_uString, ever since at least 9399c662f36c385b0c705eb34e636a9aec450282
    "initial import".)
    
    The plan is as follows:  In step 1, add temporary static_asserts that state the
    current alignof/sizeof values; keep this for a while to see that all relevant
    Windows builds actually agree on these status-quo values.  In step 2, remove the
    pragma pack cargo cult; keep the static_asserts for a while to see that the
    removal has no impact on any of the relevant Windows builds.  Finally, in
    step 3, remove the temporary static_asserts again.
    
    Change-Id: I8059ac300cc5b517db4a575f0eaba48678966537
    Reviewed-on: https://gerrit.libreoffice.org/c/core/+/114540
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/sal/rtl/string.cxx b/sal/rtl/string.cxx
index a649406f68ad..94be8029032a 100644
--- a/sal/rtl/string.cxx
+++ b/sal/rtl/string.cxx
@@ -33,6 +33,13 @@
 
 #include <rtl/math.h>
 
+#if defined _WIN32
+// Temporary check to verify that the #pragma pack around rtl_String is indeed cargo cult and can
+// safely be removed:
+static_assert(alignof (rtl_String) == 4);
+static_assert(sizeof (rtl_String) == 12);
+#endif
+
 /* ======================================================================= */
 
 #if USE_SDT_PROBES
diff --git a/sal/rtl/ustring.cxx b/sal/rtl/ustring.cxx
index a5efa62d759c..4aac64556666 100644
--- a/sal/rtl/ustring.cxx
+++ b/sal/rtl/ustring.cxx
@@ -42,6 +42,13 @@
 
 #include <rtl/math.h>
 
+#if defined _WIN32
+// Temporary check to verify that the #pragma pack around rtl_uString is indeed cargo cult and can
+// safely be removed:
+static_assert(alignof (rtl_uString) == 4);
+static_assert(sizeof (rtl_uString) == 12);
+#endif
+
 /* ======================================================================= */
 
 #if USE_SDT_PROBES


More information about the Libreoffice-commits mailing list