[PATCH xf86-input-libinput] Use attribute((cleanup)) for most of the xf86SetStrOption handling

Fri Nov 11 08:36:30 UTC 2016

Hi,

On 11-11-16 02:08, Peter Hutterer wrote:
> Code was already clean, but this removes error path handling.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> Somewhat in two minds about it because it doesn't gain us that much in this
> codebase. I want to start using this more because it is quite useful and
> this is a low-key project to start with :)

I'm not really a fan of using these gcc specific __attribute__ thingies.

Regards,

Hanse

>
>  src/xf86libinput.c | 51 +++++++++++++++++++++------------------------------
>  1 file changed, 21 insertions(+), 30 deletions(-)
>
> diff --git a/src/xf86libinput.c b/src/xf86libinput.c
> index a6481bc..5f7a551 100644
> --- a/src/xf86libinput.c
> +++ b/src/xf86libinput.c
> @@ -84,6 +84,10 @@
>  #define CAP_TABLET_TOOL	0x10
>  #define CAP_TABLET_PAD	0x20
>
> +static inline void freep(void *p) { free(*(void**)p); }
> +#define _cleanup_(x_) __attribute__((cleanup(x_)))
> +#define _autofree_ __attribute__((cleanup(freep)))
> +
>  struct xf86libinput_driver {
>  	struct libinput *libinput;
>  	int device_enabled_count;
> @@ -236,12 +240,11 @@ btn_xorg2linux(unsigned int b)
>  static BOOL
>  xf86libinput_is_subdevice(InputInfoPtr pInfo)
>  {
> -	char *source;
> +	_autofree_ char *source = NULL;
>  	BOOL is_subdevice;
>
>  	source = xf86SetStrOption(pInfo->options, "_source", "");
>  	is_subdevice = strcmp(source, "_driver/libinput") == 0;
> -	free(source);
>
>  	return is_subdevice;
>  }
> @@ -1134,12 +1137,11 @@ xf86libinput_init(DeviceIntPtr dev)
>  static bool
>  is_libinput_device(InputInfoPtr pInfo)
>  {
> -	char *driver;
> +	_autofree_ char *driver = NULL;
>  	BOOL rc;
>
>  	driver = xf86CheckStrOption(pInfo->options, "driver", "");
>  	rc = strcmp(driver, "libinput") == 0;
> -	free(driver);
>
>  	return rc;
>  }
> @@ -2012,13 +2014,12 @@ open_restricted(const char *path, int flags, void *data)
>  	int fd = -1;
>
>  	nt_list_for_each_entry(pInfo, xf86FirstLocalDevice(), next) {
> -		char *device = xf86CheckStrOption(pInfo->options, "Device", NULL);
> +		_autofree_ char *device = NULL;
>
> -		if (device != NULL && strcmp(path, device) == 0) {
> -			free(device);
> +		device = xf86CheckStrOption(pInfo->options, "Device", NULL);
> +
> +		if (device != NULL && strcmp(path, device) == 0)
>  			break;
> -		}
> -		free(device);
>  	}
>
>  	if (pInfo == NULL) {
> @@ -2176,7 +2177,7 @@ xf86libinput_parse_tap_buttonmap_option(InputInfoPtr pInfo,
>  					struct libinput_device *device)
>  {
>  	enum libinput_config_tap_button_map map;
> -	char *str;
> +	_autofree_ char *str = NULL;
>
>  	if (libinput_device_config_tap_get_finger_count(device) == 0)
>  		return FALSE;
> @@ -2194,7 +2195,6 @@ xf86libinput_parse_tap_buttonmap_option(InputInfoPtr pInfo,
>  			xf86IDrvMsg(pInfo, X_ERROR,
>  				    "Invalid TapButtonMap: %s\n",
>  				    str);
> -		free(str);
>  	}
>
>  	if (libinput_device_config_tap_set_button_map(device, map) !=
> @@ -2236,7 +2236,7 @@ xf86libinput_parse_accel_profile_option(InputInfoPtr pInfo,
>  					struct libinput_device *device)
>  {
>  	enum libinput_config_accel_profile profile;
> -	char *str;
> +	_autofree_ char *str = NULL;
>
>  	if (libinput_device_config_accel_get_profiles(device) ==
>  	    LIBINPUT_CONFIG_ACCEL_PROFILE_NONE)
> @@ -2256,8 +2256,6 @@ xf86libinput_parse_accel_profile_option(InputInfoPtr pInfo,
>  		profile = libinput_device_config_accel_get_profile(device);
>  	}
>
> -	free(str);
> -
>  	return profile;
>  }
>
> @@ -2290,7 +2288,7 @@ static inline enum libinput_config_send_events_mode
>  xf86libinput_parse_sendevents_option(InputInfoPtr pInfo,
>  				     struct libinput_device *device)
>  {
> -	char *modestr;
> +	_autofree_ char *modestr = NULL;
>  	enum libinput_config_send_events_mode mode;
>
>  	if (libinput_device_config_send_events_get_modes(device) == LIBINPUT_CONFIG_SEND_EVENTS_ENABLED)
> @@ -2311,7 +2309,6 @@ xf86libinput_parse_sendevents_option(InputInfoPtr pInfo,
>  			xf86IDrvMsg(pInfo, X_ERROR,
>  				    "Invalid SendeventsMode: %s\n",
>  				    modestr);
> -		free(modestr);
>  	}
>
>  	if (libinput_device_config_send_events_set_mode(device, mode) !=
> @@ -2329,7 +2326,7 @@ xf86libinput_parse_calibration_option(InputInfoPtr pInfo,
>  				      struct libinput_device *device,
>  				      float matrix_out[9])
>  {
> -	char *str;
> +	_autofree_ char *str = NULL;
>  	float matrix[9] = { 1.0, 0.0, 0.0, 0.0, 1.0, 0.0, 0.0, 0.0, 1.0};
>  	int num_calibration;
>
> @@ -2363,7 +2360,6 @@ xf86libinput_parse_calibration_option(InputInfoPtr pInfo,
>  	} else
>  		xf86IDrvMsg(pInfo, X_ERROR,
>  			    "Failed to apply matrix: %s, using default\n",  str);
> -	free(str);
>  }
>
>  static inline BOOL
> @@ -2396,7 +2392,7 @@ xf86libinput_parse_scroll_option(InputInfoPtr pInfo,
>  {
>  	uint32_t scroll_methods;
>  	enum libinput_config_scroll_method m;
> -	char *method;
> +	_autofree_ char *method = NULL;
>
>  	scroll_methods = libinput_device_config_scroll_get_methods(device);
>  	if (scroll_methods == LIBINPUT_CONFIG_SCROLL_NO_SCROLL)
> @@ -2420,7 +2416,6 @@ xf86libinput_parse_scroll_option(InputInfoPtr pInfo,
>  		m = libinput_device_config_scroll_get_method(device);
>  	}
>
> -	free(method);
>  	return m;
>  }
>
> @@ -2458,7 +2453,7 @@ xf86libinput_parse_clickmethod_option(InputInfoPtr pInfo,
>  {
>  	uint32_t click_methods = libinput_device_config_click_get_methods(device);
>  	enum libinput_config_click_method m;
> -	char *method;
> +	_autofree_ char *method = NULL;
>
>  	if (click_methods == LIBINPUT_CONFIG_CLICK_METHOD_NONE)
>  		return LIBINPUT_CONFIG_CLICK_METHOD_NONE;
> @@ -2479,7 +2474,6 @@ xf86libinput_parse_clickmethod_option(InputInfoPtr pInfo,
>  			    method);
>  		m = libinput_device_config_click_get_method(device);
>  	}
> -	free(method);
>
>  	return m;
>  }
> @@ -2536,7 +2530,8 @@ xf86libinput_parse_buttonmap_option(InputInfoPtr pInfo,
>  				    size_t size)
>  {
>  	const int MAXBUTTONS = 32;
> -	char *mapping, *map, *s = NULL;
> +	_autofree_ char *mapping = NULL;
> +	char *map, *s = NULL;
>  	int idx = 1;
>
>  	init_button_map(btnmap, size);
> @@ -2561,22 +2556,19 @@ xf86libinput_parse_buttonmap_option(InputInfoPtr pInfo,
>  		btnmap[idx++] = btn;
>  		map = s;
>  	} while (s && *s != '\0' && idx < MAXBUTTONS);
> -
> -	free(mapping);
>  }
>
>  static inline void
>  xf86libinput_parse_draglock_option(InputInfoPtr pInfo,
>  				   struct xf86libinput *driver_data)
>  {
> -	char *str;
> +	_autofree_ char *str = NULL;
>
>  	str = xf86CheckStrOption(pInfo->options, "DragLockButtons",NULL);
>  	if (draglock_init_from_string(&driver_data->draglock, str) != 0)
>  		xf86IDrvMsg(pInfo, X_ERROR,
>  			    "Invalid DragLockButtons option: \"%s\"\n",
>  			    str);
> -	free(str);
>  }
>
>  static inline BOOL
> @@ -2851,7 +2843,7 @@ xf86libinput_pre_init(InputDriverPtr drv,
>  	struct xf86libinput_device *shared_device = NULL;
>  	struct libinput *libinput = NULL;
>  	struct libinput_device *device;
> -	char *path = NULL;
> +	_autofree_ char *path = NULL;
>  	bool is_subdevice;
>
>  	pInfo->type_name = 0;
> @@ -2926,7 +2918,7 @@ xf86libinput_pre_init(InputDriverPtr drv,
>  	driver_data->pInfo = pInfo;
>  	driver_data->scroll.vdist = 15;
>  	driver_data->scroll.hdist = 15;
> -	driver_data->path = path;
> +	driver_data->path = strdup(path);
>  	driver_data->shared_device = shared_device;
>  	xorg_list_append(&driver_data->shared_device_link,
>  			 &shared_device->device_list);
> @@ -2977,7 +2969,6 @@ fail:
>  		if (driver_data->valuators_unaccelerated)
>  			valuator_mask_free(&driver_data->valuators_unaccelerated);
>  	}
> -	free(path);
>  	if (shared_device)
>  		xf86libinput_shared_unref(shared_device);
>  	free(driver_data);
>

[PATCH xf86-input-libinput] Use __attribute__((cleanup)) for most of the xf86SetStrOption handling

[PATCH xf86-input-libinput] Use attribute((cleanup)) for most of the xf86SetStrOption handling