GNOME Bugzilla – Bug 782549
Bugs in #781572 porting to C
Last modified: 2017-05-14 10:14:54 UTC
A few things were broken, and a few memory leaks were introduced during the https://bugzilla.gnome.org/show_bug.cgi?id=781572 ports to C. This bug is a collection for all porting bugs for 781572.
https://git.gnome.org/browse/gnome-games/tree/src/gamepad/gamepad-mapping.c #316: NULL check instead of empty check #440 #442 #444: NULL checks are inverted #7 #8: local includes
https://git.gnome.org/browse/gnome-games/tree/src/gamepad/gamepad-monitor.c #7 #8 #9 #10: global includes
In gamepqd-mapping.c in the games_gamepad_mapping_get_dpad_mapping() function, a copy of the DPad object is made into the dpad variable, we want the changes to be applied on the original hence a reference (pointer) would work better here.
Created attachment 351752 [details] [review] gamepad: Save changes made to the DPad's state Take a reference to the DPad object rather than making a copy in games_gamepad_mapping_get_dpad_mapping() so that the cahnges or saved.
Created attachment 351753 [details] [review] gamepad: Fix NULL checks in gamepadmapping destructor We want to free non-NULL pointers, not NULL ones.
Created attachment 351754 [details] [review] gamepad: Fix empty string test Test if a string is empty instead of NULL as it was the intended test in games_gamepad_mapping_new_from_sdl_string().
Created attachment 351755 [details] [review] gamepad: Fix global includes Use <> for some global includes instead of "".
Attachment 351752 [details] pushed as dfba6a5 - gamepad: Save changes made to the DPad's state Attachment 351753 [details] pushed as 18d6836 - gamepad: Fix NULL checks in gamepadmapping destructor Attachment 351754 [details] pushed as 91fd301 - gamepad: Fix empty string test Attachment 351755 [details] pushed as d5190e8 - gamepad: Fix global includes
Created attachment 351808 [details] [review] gamepad: Fix local includes Use "" for some local includes instead of <>.
Review of attachment 351808 [details] [review]: Thanks, though there is an error and we could fix things further. :) ::: src/gamepad/gamepad-monitor.c @@ +5,3 @@ #include <glib.h> #include <glib-object.h> +#include "linux-raw-gamepad-monitor.h" Shouldn't it be "linux/linux-raw-gamepad-monitor.h" instead? It worked with <> because the Linux dir is in the include paths. @@ +8,3 @@ +#include "raw-gamepad-monitor.h" +#include "raw-gamepad.h" +#include "gamepad-mapping-error.h" Let's take it as an occasion to move this one first to sort them alphabetically.
Created attachment 351810 [details] [review] gamepad: Fix local includes Use "" for some local includes instead of <>, and reorder the includes alphabetically.
Attachment 351810 [details] pushed as b4cfcc9 - gamepad: Fix local includes