ucb/source/ucp/gio/gio_mount.cxx | 35 +++++++++++++---------------------- ucb/source/ucp/gio/gio_mount.hxx | 13 ++++++++----- 2 files changed, 21 insertions(+), 27 deletions(-)
New commits: commit 591d57a21e0f1b4641095d4fcd66c44320d272ed Author: Noel Grandin <[email protected]> AuthorDate: Tue Mar 4 15:45:45 2025 +0200 Commit: Noel Grandin <[email protected]> CommitDate: Wed Mar 5 17:10:12 2025 +0100 fix dodgy lifecycle in OOoMountOperation We fix/improve 3 things here (1) Holding a pointer to something that might be on the stack seems dodgy. I traced the calls back a few levels and ended up with lots of lots of call sites, with no guarantee this pointer is safe to hold for any length of time. (2) we already have a unique_ptr in this structure, but we are not properly constructing it. (3) once we fix (1) and (2) by calling the constructor/destructor explicitly, we can simply the password/username field handling. This code has been here since: commit 065b374d695f5208c6dcdad8ead3f1ff6223036f Author: Vladimir Glazounov <[email protected]> Date: Wed May 14 13:49:20 2008 +0000 INTEGRATION: CWS ucpgio1 (1.1.2); FILE ADDED 2008/04/30 16:35:19 cmc 1.1.2.1: #i88090# add gio content provider Change-Id: I3ffcf30fbf66c855cedc4a3a99985bc6ee42faa9 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/182483 Reviewed-by: Caolán McNamara <[email protected]> Reviewed-by: Noel Grandin <[email protected]> Tested-by: Jenkins diff --git a/ucb/source/ucp/gio/gio_mount.cxx b/ucb/source/ucp/gio/gio_mount.cxx index 11e4117deb82..17b961e4857d 100644 --- a/ucb/source/ucp/gio/gio_mount.cxx +++ b/ucb/source/ucp/gio/gio_mount.cxx @@ -45,18 +45,15 @@ static void ooo_mount_operation_ask_password (GMountOperation *op, static void ooo_mount_operation_init (OOoMountOperation *op) { - op->m_pPrevPassword = nullptr; - op->m_pPrevUsername = nullptr; + op->m_aPrevPassword.clear(); + op->m_aPrevUsername.clear(); } -static void ooo_mount_operation_finalize (GObject *object) +void ooo_mount_operation_finalize (GObject *object) { OOoMountOperation *mount_op = OOO_MOUNT_OPERATION (object); - if (mount_op->m_pPrevUsername) - free(mount_op->m_pPrevUsername); - if (mount_op->m_pPrevPassword) - free(mount_op->m_pPrevPassword); - mount_op->context.reset(); + + mount_op->~OOoMountOperation(); G_OBJECT_CLASS (ooo_mount_operation_parent_class)->finalize (object); } @@ -96,7 +93,7 @@ static void ooo_mount_operation_ask_password (GMountOperation *op, OOoMountOperation *pThis = reinterpret_cast<OOoMountOperation*>(op); GlibThreadDefaultMainContextScope scope(pThis->context.get()); - const css::uno::Reference< css::ucb::XCommandEnvironment > &xEnv = *(pThis->pEnv); + const css::uno::Reference< css::ucb::XCommandEnvironment > &xEnv = pThis->xEnv; if (xEnv.is()) xIH = xEnv->getInteractionHandler(); @@ -123,11 +120,8 @@ static void ooo_mount_operation_ask_password (GMountOperation *op, ? ucbhelper::SimpleAuthenticationRequest::ENTITY_MODIFY : ucbhelper::SimpleAuthenticationRequest::ENTITY_NA; - OUString aPrevPassword, aPrevUsername; - if (pThis->m_pPrevUsername) - aPrevUsername = OUString(pThis->m_pPrevUsername, strlen(pThis->m_pPrevUsername), RTL_TEXTENCODING_UTF8); - if (pThis->m_pPrevPassword) - aPrevPassword = OUString(pThis->m_pPrevPassword, strlen(pThis->m_pPrevPassword), RTL_TEXTENCODING_UTF8); + OUString aPrevPassword = pThis->m_aPrevUsername; + OUString aPrevUsername = pThis->m_aPrevPassword; //The damn dialog is stupidly broken, so do like webdav, i.e. "#102871#" if ( aUserName.isEmpty() ) @@ -191,20 +185,17 @@ static void ooo_mount_operation_ask_password (GMountOperation *op, break; } - if (pThis->m_pPrevPassword) - free(pThis->m_pPrevPassword); - pThis->m_pPrevPassword = strdup(OUStringToOString(aPassword, RTL_TEXTENCODING_UTF8).getStr()); - if (pThis->m_pPrevUsername) - free(pThis->m_pPrevUsername); - pThis->m_pPrevUsername = strdup(OUStringToOString(aUserName, RTL_TEXTENCODING_UTF8).getStr()); + pThis->m_aPrevPassword = aPassword; + pThis->m_aPrevUsername = aUserName; g_mount_operation_reply (op, G_MOUNT_OPERATION_HANDLED); } GMountOperation *ooo_mount_operation_new(ucb::ucp::gio::glib::MainContextRef && context, const css::uno::Reference< css::ucb::XCommandEnvironment >& rEnv) { - OOoMountOperation *pRet = static_cast<OOoMountOperation*>(g_object_new (OOO_TYPE_MOUNT_OPERATION, nullptr)); + void* pMem = g_object_new (OOO_TYPE_MOUNT_OPERATION, nullptr); + OOoMountOperation *pRet = new (pMem) OOoMountOperation; pRet->context = std::move(context); - pRet->pEnv = &rEnv; + pRet->xEnv = rEnv; return &pRet->parent_instance; } diff --git a/ucb/source/ucp/gio/gio_mount.hxx b/ucb/source/ucp/gio/gio_mount.hxx index 2ddcaebf5795..23c3bfe513be 100644 --- a/ucb/source/ucp/gio/gio_mount.hxx +++ b/ucb/source/ucp/gio/gio_mount.hxx @@ -55,17 +55,20 @@ using MainContextRef = std::unique_ptr<GMainContext, detail::MainContextUnref>; struct OOoMountOperation { + friend GMountOperation *ooo_mount_operation_new(ucb::ucp::gio::glib::MainContextRef &&, const css::uno::Reference< css::ucb::XCommandEnvironment >&); + friend void ooo_mount_operation_finalize(GObject *); + GMountOperation parent_instance; ucb::ucp::gio::glib::MainContextRef context; - const css::uno::Reference< css::ucb::XCommandEnvironment > *pEnv; - char *m_pPrevUsername; - char *m_pPrevPassword; + css::uno::Reference< css::ucb::XCommandEnvironment > xEnv; + OUString m_aPrevUsername; + OUString m_aPrevPassword; private: // Managed via ooo_mount_operation_new and ooo_mount_operation_finalize: - OOoMountOperation() = delete; - ~OOoMountOperation() = delete; + OOoMountOperation() = default; + ~OOoMountOperation() = default; }; struct OOoMountOperationClass
