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

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

Changes from 2 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
13 changes: 3 additions & 10 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 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)
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 @@ -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 @@ -133,6 +131,7 @@ battstat_upower_cleanup( void )
if( upc == NULL )
return;

alwilson marked this conversation as resolved.
Show resolved Hide resolved
g_object_unref( devices );
Copy link

Choose a reason for hiding this comment

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

I'd re-do the code here to put unref calls into if (ptr != NULL) ... unref()
Besides, call g_ptr_array_unref for safety. Hell knows how GTK works.

Copy link
Author

Choose a reason for hiding this comment

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

This is my first time touching GTK, so I'll take your word on that. 😋

Copy link

Choose a reason for hiding this comment

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

This is my first time reviewing GTK, so best of luck.
Hell, all the code seems the same after the years.

Copy link

Choose a reason for hiding this comment

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

You must be glad that I at least have learned some C :trollface:

g_object_unref( upc );
upc = NULL;
}
Expand All @@ -154,9 +153,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 +255,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 +321,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