[PATCH v2 3/7] input: un-constify InputAttributes
Hans de Goede
hdegoede at redhat.com
Tue Feb 4 09:16:16 CET 2014
Hi,
On 02/04/2014 02:34 AM, Peter Hutterer wrote:
> Introduced in fecc7eb1cf66db64728ee2d68cd9443df7e70879 and reverts most of
> that but it's helpfully mixed with other stuff.
>
> InputAttributes are not const, they're strdup'd everywhere but the test code
> and freed properly. Revert the const char changes and fix the test up instead.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> I thought I did run the tests after this patch but apparently I didn't.
> FreeInputAttributes caused an invalid free because one struct in the tests
> was on the stack.
>
> Changes to v1:
> - use xnfstrdup, not strdup
> - now that we call FreeInputAttributes(orig) in the test, that struct must
> be allocated on the heap, since FIA will free it.
>
> The latter looks like a big change (last hunk) but it's really just a struct
> vs pointer replacement.
> Hans, your rev-by still good for this?
Just re-reviewed it, and yep this is still:
Reviewed-by: Hans de Goede <hdegoede at redhat.com>
Regards,
Hans
>
> config/udev.c | 14 +++++++-------
> dix/inpututils.c | 16 ++++++++--------
> include/input.h | 12 ++++++------
> test/input.c | 56 ++++++++++++++++++++++++++++++--------------------------
> 4 files changed, 51 insertions(+), 47 deletions(-)
>
> diff --git a/config/udev.c b/config/udev.c
> index 436b8f0..68ed348 100644
> --- a/config/udev.c
> +++ b/config/udev.c
> @@ -242,16 +242,16 @@ device_added(struct udev_device *udev_device)
> free(config_info);
> input_option_free_list(&input_options);
>
> - free((void *) attrs.usb_id);
> - free((void *) attrs.pnp_id);
> - free((void *) attrs.product);
> - free((void *) attrs.device);
> - free((void *) attrs.vendor);
> + free(attrs.usb_id);
> + free(attrs.pnp_id);
> + free(attrs.product);
> + free(attrs.device);
> + free(attrs.vendor);
> if (attrs.tags) {
> - const char **tag = attrs.tags;
> + char **tag = attrs.tags;
>
> while (*tag) {
> - free((void *) *tag);
> + free(*tag);
> tag++;
> }
> free(attrs.tags);
> diff --git a/dix/inpututils.c b/dix/inpututils.c
> index 3e1d75f..a10a7c7 100644
> --- a/dix/inpututils.c
> +++ b/dix/inpututils.c
> @@ -351,7 +351,7 @@ DuplicateInputAttributes(InputAttributes * attrs)
> {
> InputAttributes *new_attr;
> int ntags = 0;
> - const char **tags, **new_tags;
> + char **tags, **new_tags;
>
> if (!attrs)
> return NULL;
> @@ -403,20 +403,20 @@ DuplicateInputAttributes(InputAttributes * attrs)
> void
> FreeInputAttributes(InputAttributes * attrs)
> {
> - const char **tags;
> + char **tags;
>
> if (!attrs)
> return;
>
> - free((void *) attrs->product);
> - free((void *) attrs->vendor);
> - free((void *) attrs->device);
> - free((void *) attrs->pnp_id);
> - free((void *) attrs->usb_id);
> + free(attrs->product);
> + free(attrs->vendor);
> + free(attrs->device);
> + free(attrs->pnp_id);
> + free(attrs->usb_id);
>
> if ((tags = attrs->tags))
> while (*tags)
> - free((void *) *tags++);
> + free(*tags++);
>
> free(attrs->tags);
> free(attrs);
> diff --git a/include/input.h b/include/input.h
> index 455963f..6f047ee 100644
> --- a/include/input.h
> +++ b/include/input.h
> @@ -221,12 +221,12 @@ typedef struct _InputOption InputOption;
> typedef struct _XI2Mask XI2Mask;
>
> typedef struct _InputAttributes {
> - const char *product;
> - const char *vendor;
> - const char *device;
> - const char *pnp_id;
> - const char *usb_id;
> - const char **tags; /* null-terminated */
> + char *product;
> + char *vendor;
> + char *device;
> + char *pnp_id;
> + char *usb_id;
> + char **tags; /* null-terminated */
> uint32_t flags;
> } InputAttributes;
>
> diff --git a/test/input.c b/test/input.c
> index aaa7a69..5813e6d 100644
> --- a/test/input.c
> +++ b/test/input.c
> @@ -1101,7 +1101,7 @@ xi_unregister_handlers(void)
> static void
> cmp_attr_fields(InputAttributes * attr1, InputAttributes * attr2)
> {
> - const char **tags1, **tags2;
> + char **tags1, **tags2;
>
> assert(attr1 && attr2);
> assert(attr1 != attr2);
> @@ -1180,50 +1180,54 @@ cmp_attr_fields(InputAttributes * attr1, InputAttributes * attr2)
> static void
> dix_input_attributes(void)
> {
> - InputAttributes orig = { 0 };
> + InputAttributes *orig;
> InputAttributes *new;
> - const char *tags[4] = { "tag1", "tag2", "tag2", NULL };
>
> new = DuplicateInputAttributes(NULL);
> assert(!new);
>
> - new = DuplicateInputAttributes(&orig);
> - assert(memcmp(&orig, new, sizeof(InputAttributes)) == 0);
> + orig = calloc(1, sizeof(InputAttributes));
> + assert(orig);
>
> - orig.product = "product name";
> - new = DuplicateInputAttributes(&orig);
> - cmp_attr_fields(&orig, new);
> + new = DuplicateInputAttributes(orig);
> + assert(memcmp(orig, new, sizeof(InputAttributes)) == 0);
> +
> + orig->product = xnfstrdup("product name");
> + new = DuplicateInputAttributes(orig);
> + cmp_attr_fields(orig, new);
> FreeInputAttributes(new);
>
> - orig.vendor = "vendor name";
> - new = DuplicateInputAttributes(&orig);
> - cmp_attr_fields(&orig, new);
> + orig->vendor = xnfstrdup("vendor name");
> + new = DuplicateInputAttributes(orig);
> + cmp_attr_fields(orig, new);
> FreeInputAttributes(new);
>
> - orig.device = "device path";
> - new = DuplicateInputAttributes(&orig);
> - cmp_attr_fields(&orig, new);
> + orig->device = xnfstrdup("device path");
> + new = DuplicateInputAttributes(orig);
> + cmp_attr_fields(orig, new);
> FreeInputAttributes(new);
>
> - orig.pnp_id = "PnPID";
> - new = DuplicateInputAttributes(&orig);
> - cmp_attr_fields(&orig, new);
> + orig->pnp_id = xnfstrdup("PnPID");
> + new = DuplicateInputAttributes(orig);
> + cmp_attr_fields(orig, new);
> FreeInputAttributes(new);
>
> - orig.usb_id = "USBID";
> - new = DuplicateInputAttributes(&orig);
> - cmp_attr_fields(&orig, new);
> + orig->usb_id = xnfstrdup("USBID");
> + new = DuplicateInputAttributes(orig);
> + cmp_attr_fields(orig, new);
> FreeInputAttributes(new);
>
> - orig.flags = 0xF0;
> - new = DuplicateInputAttributes(&orig);
> - cmp_attr_fields(&orig, new);
> + orig->flags = 0xF0;
> + new = DuplicateInputAttributes(orig);
> + cmp_attr_fields(orig, new);
> FreeInputAttributes(new);
>
> - orig.tags = tags;
> - new = DuplicateInputAttributes(&orig);
> - cmp_attr_fields(&orig, new);
> + orig->tags = xstrtokenize("tag1 tag2 tag3", " ");
> + new = DuplicateInputAttributes(orig);
> + cmp_attr_fields(orig, new);
> FreeInputAttributes(new);
> +
> + FreeInputAttributes(orig);
> }
>
> static void
>
More information about the xorg-devel
mailing list