ppp: cleanup nm-ppp-manager-call to use const pointer and atomic operations

- Mark NMPPPOps variable as const. It really must not be modified.

- We cache the loaded symbols in a global variable. While this code
  is not used in a multi threaded situation, I think we should not
  add code that uses global variables that is not thread safe.

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/837
This commit is contained in:
Thomas Haller 2021-04-21 12:11:01 +02:00
parent f3821b27dd
commit 037a94e837
No known key found for this signature in database
GPG key ID: 29C2366E4DFC5728
2 changed files with 31 additions and 16 deletions

View file

@ -18,17 +18,21 @@
/*****************************************************************************/
static NMPPPOps *ppp_ops = NULL;
static const NMPPPOps *_ppp_ops = NULL;
#define ppp_ops_get() ((const NMPPPOps *) g_atomic_pointer_get(&_ppp_ops))
NMPPPManager *
nm_ppp_manager_create(const char *iface, GError **error)
{
NMPPPManager *ret;
GModule * plugin;
GError * error_local = NULL;
NMPPPOps * ops;
struct stat st;
NMPPPManager * ret;
GModule * plugin;
GError * error_local = NULL;
struct stat st;
const NMPPPOps *ppp_ops;
again:
ppp_ops = ppp_ops_get();
if (G_UNLIKELY(!ppp_ops)) {
if (stat(PPP_PLUGIN_PATH, &st) != 0) {
g_set_error_literal(error,
@ -58,7 +62,7 @@ nm_ppp_manager_create(const char *iface, GError **error)
return NULL;
}
if (!g_module_symbol(plugin, "ppp_ops", (gpointer) &ops)) {
if (!g_module_symbol(plugin, "ppp_ops", (gpointer *) &ppp_ops)) {
g_set_error(error,
NM_MANAGER_ERROR,
NM_MANAGER_ERROR_MISSING_PLUGIN,
@ -67,18 +71,21 @@ nm_ppp_manager_create(const char *iface, GError **error)
return NULL;
}
nm_assert(ppp_ops);
nm_assert(ppp_ops->create);
nm_assert(ppp_ops->start);
nm_assert(ppp_ops->stop);
nm_assert(ppp_ops->stop_cancel);
if (!g_atomic_pointer_compare_and_exchange(&_ppp_ops, NULL, ppp_ops)) {
g_module_close(plugin);
goto again;
}
/* after loading glib types from the plugin, we cannot unload the library anymore.
* Make it resident. */
g_module_make_resident(plugin);
nm_assert(ops);
nm_assert(ops->create);
nm_assert(ops->start);
nm_assert(ops->stop);
nm_assert(ops->stop_cancel);
ppp_ops = ops;
nm_log_info(LOGD_CORE | LOGD_PPP, "loaded PPP plugin " PPP_PLUGIN_PATH);
}
@ -94,6 +101,8 @@ nm_ppp_manager_set_route_parameters(NMPPPManager *self,
guint32 ip6_route_table,
guint32 ip6_route_metric)
{
const NMPPPOps *ppp_ops = ppp_ops_get();
g_return_if_fail(ppp_ops);
ppp_ops->set_route_parameters(self,
@ -111,6 +120,8 @@ nm_ppp_manager_start(NMPPPManager *self,
guint baud_override,
GError ** err)
{
const NMPPPOps *ppp_ops = ppp_ops_get();
g_return_val_if_fail(ppp_ops, FALSE);
return ppp_ops->start(self, req, ppp_name, timeout_secs, baud_override, err);
@ -122,6 +133,8 @@ nm_ppp_manager_stop(NMPPPManager * self,
NMPPPManagerStopCallback callback,
gpointer user_data)
{
const NMPPPOps *ppp_ops = ppp_ops_get();
g_return_val_if_fail(ppp_ops, NULL);
return ppp_ops->stop(self, cancellable, callback, user_data);
@ -130,6 +143,8 @@ nm_ppp_manager_stop(NMPPPManager * self,
void
nm_ppp_manager_stop_cancel(NMPPPManagerStopHandle *handle)
{
const NMPPPOps *ppp_ops = ppp_ops_get();
g_return_if_fail(ppp_ops);
g_return_if_fail(handle);

View file

@ -1450,7 +1450,7 @@ nm_ppp_manager_class_init(NMPPPManagerClass *manager_class)
G_TYPE_UINT); /* guint32 out_bytes */
}
NMPPPOps ppp_ops = {
const NMPPPOps ppp_ops = {
.create = _ppp_manager_new,
.set_route_parameters = _ppp_manager_set_route_parameters,
.start = _ppp_manager_start,