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.hutte...@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 <hdego...@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 > _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel