| Message ID | 1391039480-5676-4-git-send-email-peter.hutterer@who-t.net |
|---|---|
| State | Accepted |
| Headers | show |
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..458714f 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); @@ -1182,7 +1182,6 @@ dix_input_attributes(void) { InputAttributes orig = { 0 }; InputAttributes *new; - const char *tags[4] = { "tag1", "tag2", "tag2", NULL }; new = DuplicateInputAttributes(NULL); assert(!new); @@ -1190,27 +1189,27 @@ dix_input_attributes(void) new = DuplicateInputAttributes(&orig); assert(memcmp(&orig, new, sizeof(InputAttributes)) == 0); - orig.product = "product name"; + orig.product = strdup("product name"); new = DuplicateInputAttributes(&orig); cmp_attr_fields(&orig, new); FreeInputAttributes(new); - orig.vendor = "vendor name"; + orig.vendor = strdup("vendor name"); new = DuplicateInputAttributes(&orig); cmp_attr_fields(&orig, new); FreeInputAttributes(new); - orig.device = "device path"; + orig.device = strdup("device path"); new = DuplicateInputAttributes(&orig); cmp_attr_fields(&orig, new); FreeInputAttributes(new); - orig.pnp_id = "PnPID"; + orig.pnp_id = strdup("PnPID"); new = DuplicateInputAttributes(&orig); cmp_attr_fields(&orig, new); FreeInputAttributes(new); - orig.usb_id = "USBID"; + orig.usb_id = strdup("USBID"); new = DuplicateInputAttributes(&orig); cmp_attr_fields(&orig, new); FreeInputAttributes(new); @@ -1220,10 +1219,12 @@ dix_input_attributes(void) cmp_attr_fields(&orig, new); FreeInputAttributes(new); - orig.tags = tags; + orig.tags = xstrtokenize("tag1 tag2 tag3", " "); new = DuplicateInputAttributes(&orig); cmp_attr_fields(&orig, new); FreeInputAttributes(new); + + FreeInputAttributes(&orig); } static void
Hi, On 01/30/2014 12:51 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@who-t.net> Please use XNFstrdup instead of strdup (or add explicit error checking) with that changed: Reviewed By: Hans de Goede <hdegoede@redhat.com> Regards, Hans > --- > config/udev.c | 14 +++++++------- > dix/inpututils.c | 16 ++++++++-------- > include/input.h | 12 ++++++------ > test/input.c | 17 +++++++++-------- > 4 files changed, 30 insertions(+), 29 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..458714f 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); > @@ -1182,7 +1182,6 @@ dix_input_attributes(void) > { > InputAttributes orig = { 0 }; > InputAttributes *new; > - const char *tags[4] = { "tag1", "tag2", "tag2", NULL }; > > new = DuplicateInputAttributes(NULL); > assert(!new); > @@ -1190,27 +1189,27 @@ dix_input_attributes(void) > new = DuplicateInputAttributes(&orig); > assert(memcmp(&orig, new, sizeof(InputAttributes)) == 0); > > - orig.product = "product name"; > + orig.product = strdup("product name"); > new = DuplicateInputAttributes(&orig); > cmp_attr_fields(&orig, new); > FreeInputAttributes(new); > > - orig.vendor = "vendor name"; > + orig.vendor = strdup("vendor name"); > new = DuplicateInputAttributes(&orig); > cmp_attr_fields(&orig, new); > FreeInputAttributes(new); > > - orig.device = "device path"; > + orig.device = strdup("device path"); > new = DuplicateInputAttributes(&orig); > cmp_attr_fields(&orig, new); > FreeInputAttributes(new); > > - orig.pnp_id = "PnPID"; > + orig.pnp_id = strdup("PnPID"); > new = DuplicateInputAttributes(&orig); > cmp_attr_fields(&orig, new); > FreeInputAttributes(new); > > - orig.usb_id = "USBID"; > + orig.usb_id = strdup("USBID"); > new = DuplicateInputAttributes(&orig); > cmp_attr_fields(&orig, new); > FreeInputAttributes(new); > @@ -1220,10 +1219,12 @@ dix_input_attributes(void) > cmp_attr_fields(&orig, new); > FreeInputAttributes(new); > > - orig.tags = tags; > + orig.tags = xstrtokenize("tag1 tag2 tag3", " "); > new = DuplicateInputAttributes(&orig); > cmp_attr_fields(&orig, new); > FreeInputAttributes(new); > + > + FreeInputAttributes(&orig); > } > > static void >
On Thu, Jan 30, 2014 at 09:43:37 +0100, Hans de Goede wrote: > Hi, > > On 01/30/2014 12:51 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@who-t.net> > > Please use XNFstrdup instead of strdup (or add explicit error checking) > with that changed: > AFAICT the only strdups Peter adds are in the test code. I don't think it matters there. Cheers, Julien
Hi, On 01/30/2014 02:26 PM, Julien Cristau wrote: > On Thu, Jan 30, 2014 at 09:43:37 +0100, Hans de Goede wrote: > >> Hi, >> >> On 01/30/2014 12:51 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@who-t.net> >> >> Please use XNFstrdup instead of strdup (or add explicit error checking) >> with that changed: >> > AFAICT the only strdups Peter adds are in the test code. I don't think > it matters there. Look closer, he also adds strdup calls to the new XkbInitRules function in xkb/xkbInit.c . Regards, Hans
Hi, On 01/30/2014 02:54 PM, Hans de Goede wrote: > Hi, > > On 01/30/2014 02:26 PM, Julien Cristau wrote: >> On Thu, Jan 30, 2014 at 09:43:37 +0100, Hans de Goede wrote: >> >>> Hi, >>> >>> On 01/30/2014 12:51 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@who-t.net> >>> >>> Please use XNFstrdup instead of strdup (or add explicit error checking) >>> with that changed: >>> >> AFAICT the only strdups Peter adds are in the test code. I don't think >> it matters there. > > Look closer, he also adds strdup calls to the new XkbInitRules function in > xkb/xkbInit.c . Oops correction I should look closer myself, that is in 2/7 not 3/7 . Anyways I still believe that using XNFstrdup instead would be better. I don't see why sloppy coding (lacking error checking is sloppy coding) would be ok in test cases. Regards, Hans
On Thu, Jan 30, 2014 at 14:57:23 +0100, Hans de Goede wrote: > Oops correction I should look closer myself, that is in 2/7 not 3/7 . Anyways > I still believe that using XNFstrdup instead would be better. I don't see why > sloppy coding (lacking error checking is sloppy coding) would be ok in test > cases. > I'd expect that either it's ok for these things to be NULL, or the test will fail when they are? Cheers, Julien
On Thu, Jan 30, 2014 at 03:04:41PM +0100, Julien Cristau wrote: > On Thu, Jan 30, 2014 at 14:57:23 +0100, Hans de Goede wrote: > > > Oops correction I should look closer myself, that is in 2/7 not 3/7 . Anyways > > I still believe that using XNFstrdup instead would be better. I don't see why > > sloppy coding (lacking error checking is sloppy coding) would be ok in test > > cases. > > > I'd expect that either it's ok for these things to be NULL, or the test > will fail when they are? correct, it'll crash on strcmp(). though xnfstrdup is simple enough to add though and it does add that little bit of value that I've just amended locally. Cheers, Peter
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@who-t.net> --- config/udev.c | 14 +++++++------- dix/inpututils.c | 16 ++++++++-------- include/input.h | 12 ++++++------ test/input.c | 17 +++++++++-------- 4 files changed, 30 insertions(+), 29 deletions(-)