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.

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

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
Changes from 1 commit
Commits
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
18 changes: 5 additions & 13 deletions battstat/battstat-upower.c
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 Down Expand Up @@ -83,9 +85,6 @@ battstat_upower_initialise (void (*callback) (void))
int i, num;

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

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 @@ -101,7 +100,6 @@ battstat_upower_initialise (void (*callback) (void))
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);
Expand Down Expand Up @@ -132,7 +130,7 @@ battstat_upower_cleanup( void )
{
if( upc == NULL )
return;
alwilson marked this conversation as resolved.
Show resolved Hide resolved

g_object_unref( upc );
upc = NULL;
}
Expand All @@ -154,9 +152,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 @@ -211,7 +206,7 @@ battstat_upower_get_battery_info( BatteryStatus *status )
int type, state;
double current_charge, full_capacity, rate;
gint64 time_to_full, time_to_empty;

g_object_get( upd,
"kind", &type,
"state", &state,
Expand Down Expand Up @@ -259,7 +254,6 @@ battstat_upower_get_battery_info( BatteryStatus *status )
status->on_ac_power = TRUE;
status->charging = FALSE;

g_ptr_array_unref( devices );
return;
}

Expand All @@ -279,7 +273,7 @@ battstat_upower_get_battery_info( BatteryStatus *status )
* (ie: the PMU or APM subsystem).
*
* upower gives remaining time in seconds with a 0 to mean that the
* remaining time is unknown. Battstat uses minutes and -1 for
* remaining time is unknown. Battstat uses minutes and -1 for
alwilson marked this conversation as resolved.
Show resolved Hide resolved
* unknown time remaining.
*/

Expand Down Expand Up @@ -326,8 +320,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