Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

battstat: call up_client_get_devices once to avoid mem leak #443

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

battstat: call up_client_get_devices once to avoid mem leak #443

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2991bc7
battstat: call up_client_get_devices once to avoid mem leak
Feb 26, 2020
a73baf2
battstat: remove whitespace noise; free devices ptr in cleanup func
Feb 26, 2020
591ebcc
battstat: use more specific unref for devices ptr; rearrange cleanup
Feb 26, 2020
135c7c4
battstat: bump upower version; update depricated funcs and remove upo…
Feb 26, 2020
be43872
add autoconf-archive pkg to arch build config
Feb 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ requires:
- which
- wireless_tools
- yelp-tools
- autoconf-archive

debian:
# Useful URL: https://github.com/mate-desktop/debian-packages
Expand Down
36 changes: 8 additions & 28 deletions battstat/battstat-upower.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ static void (*status_updated_callback) (void);
*/
static gboolean status_update_scheduled;

static GPtrArray *devices;

static gboolean
update_status_idle (gpointer junk)
{
Expand All @@ -67,12 +69,10 @@ device_cb (UpClient *client, UpDevice *device, gpointer user_data) {
schedule_status_callback();
}

#if UP_CHECK_VERSION (0, 99, 0)
static void
device_removed_cb (UpClient *client, const gchar *object_path, gpointer user_data) {
schedule_status_callback();
}
#endif

/* ---- public functions ---- */

Expand All @@ -83,9 +83,6 @@ battstat_upower_initialise (void (*callback) (void))
int i, num;

status_updated_callback = callback;
#if UP_CHECK_VERSION (0, 99, 0)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If anyone wonders, removing #if is fine as the actual application for this happens below.
Sharing the variable between functions would lead to eventual utilization of up_client_get_devices unconditionally anyway.

GPtrArray *devices;
#endif

if( upc != NULL )
return g_strdup( "Already initialised!" );
Expand All @@ -96,26 +93,13 @@ battstat_upower_initialise (void (*callback) (void))
GCancellable *cancellable = g_cancellable_new();
GError *gerror;

#if UP_CHECK_VERSION(0, 99, 0)
devices = up_client_get_devices(upc);
devices = up_client_get_devices2(upc);
if (!devices) {
goto error_shutdownclient;
}
g_ptr_array_unref(devices);
#else
if (! up_client_enumerate_devices_sync( upc, cancellable, &gerror ) ) {
sprintf(error_str, "Unable to enumerate upower devices: %s\n", gerror->message);
goto error_shutdownclient;
}
#endif

g_signal_connect_after( upc, "device-added", G_CALLBACK (device_cb), NULL );
#if UP_CHECK_VERSION(0, 99, 0)
g_signal_connect_after( upc, "device-removed", G_CALLBACK (device_removed_cb), NULL );
#else
g_signal_connect_after( upc, "device-changed", G_CALLBACK (device_cb), NULL );
g_signal_connect_after( upc, "device-removed", G_CALLBACK (device_cb), NULL );
#endif

return NULL;

Expand All @@ -130,11 +114,13 @@ battstat_upower_initialise (void (*callback) (void))
void
battstat_upower_cleanup( void )
{
if( upc == NULL )
return;
if( upc != NULL )
g_object_unref( upc );
if( devices != NULL )
g_ptr_array_unref( devices );

alwilson marked this conversation as resolved.
Show resolved Hide resolved
g_object_unref( upc );
upc = NULL;
devices = NULL;
}

#include "battstat.h"
Expand All @@ -154,9 +140,6 @@ battstat_upower_cleanup( void )
void
battstat_upower_get_battery_info( BatteryStatus *status )
{

GPtrArray *devices = up_client_get_devices( upc );

/* The calculation to get overall percentage power remaining is as follows:
*
* Sum( Current charges ) / Sum( Full Capacities )
Expand Down Expand Up @@ -259,7 +242,6 @@ battstat_upower_get_battery_info( BatteryStatus *status )
status->on_ac_power = TRUE;
status->charging = FALSE;

g_ptr_array_unref( devices );
return;
}

Expand Down Expand Up @@ -326,8 +308,6 @@ battstat_upower_get_battery_info( BatteryStatus *status )
/* These are simple and well-explained above. */
status->charging = charging;
status->on_ac_power = on_ac_power;

g_ptr_array_unref( devices );
}

void
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ GTK_REQUIRED=3.22.0
LIBPANEL4_REQUIRED=1.17.0
LIBGTOP_REQUIRED=2.12.0
LIBNOTIFY_REQUIRED=0.7.0
UPOWER_REQUIRED=0.9.4
UPOWER_REQUIRED=0.99.8
DBUS_REQUIRED=1.10.0
DBUS_GLIB_REQUIRED=0.74
LIBXML_REQUIRED=2.5.0
Expand Down