* [PATCH 2/7] ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device
@ 2008-04-16 18:52 Thomas Renninger
2008-04-17 9:02 ` Zhao Yakui
0 siblings, 1 reply; 3+ messages in thread
From: Thomas Renninger @ 2008-04-16 18:52 UTC (permalink / raw)
To: Len Brown
Cc: linux-acpi, Jonathan Woithe, Carlos Corbacho, Corentin CHARY,
Henrique de Moraes Holschuh, Mattia Dongili
ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device
Rip the functionality out of acpi_device_set_id, it is needed elsewhere in
the code on follow-up patches.
Signed-off-by: Thomas Renninger <trenn@suse.de>
---
drivers/acpi/scan.c | 97 ++++++++++++++++++++++++++--------------------------
1 file changed, 50 insertions(+), 47 deletions(-)
Index: linux-acpi-2.6_video_native_vs_vendor/drivers/acpi/scan.c
===================================================================
--- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/acpi/scan.c
+++ linux-acpi-2.6_video_native_vs_vendor/drivers/acpi/scan.c
@@ -954,6 +954,52 @@ static int acpi_dock_match(struct acpi_d
return acpi_get_handle(device->handle, "_DCK", &tmp);
}
+/* Add a single Compatible ID or a whole list of CIDs to a device */
+static void cid_add(struct acpi_device *device, const char *cid_add,
+ struct acpi_compatible_id_list *cid_list) {
+
+ struct acpi_compatible_id_list *list = NULL;
+ int size = 0;
+ int count = 0;
+
+ if (!cid_list && !cid_add)
+ return;
+
+ if (cid_list) {
+ size = cid_list->size;
+ } else if (cid_add) {
+ size = sizeof(struct acpi_compatible_id_list);
+ cid_list = ACPI_ALLOCATE_ZEROED((acpi_size) size);
+ if (!cid_list) {
+ printk(KERN_ERR "Memory allocation error\n");
+ return;
+ } else {
+ cid_list->count = 0;
+ cid_list->size = size;
+ }
+ }
+ if (cid_add)
+ size += sizeof(struct acpi_compatible_id);
+ list = kmalloc(size, GFP_KERNEL);
+
+ if (list) {
+ if (cid_list) {
+ memcpy(list, cid_list, cid_list->size);
+ count = cid_list->count;
+ }
+ if (cid_add) {
+ strncpy(list->id[count].value, cid_add,
+ ACPI_MAX_CID_LENGTH);
+ count++;
+ device->flags.compatible_ids = 1;
+ }
+ list->size = size;
+ list->count = count;
+ device->pnp.cid_list = list;
+ } else
+ printk(KERN_ERR PREFIX "Memory allocation error\n");
+}
+
static void acpi_device_set_id(struct acpi_device *device,
struct acpi_device *parent, acpi_handle handle,
int type)
@@ -962,8 +1008,6 @@ static void acpi_device_set_id(struct ac
struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
char *hid = NULL;
char *uid = NULL;
- struct acpi_compatible_id_list *cid_list = NULL;
- const char *cid_add = NULL;
acpi_status status;
switch (type) {
@@ -980,7 +1024,7 @@ static void acpi_device_set_id(struct ac
if (info->valid & ACPI_VALID_UID)
uid = info->unique_id.value;
if (info->valid & ACPI_VALID_CID)
- cid_list = &info->compatibility_id;
+ cid_add(device, NULL, &info->compatibility_id);
if (info->valid & ACPI_VALID_ADR) {
device->pnp.bus_address = info->address;
device->flags.bus_address = 1;
@@ -992,11 +1036,11 @@ static void acpi_device_set_id(struct ac
against another driver.
*/
if (ACPI_SUCCESS(acpi_video_bus_match(device)))
- cid_add = ACPI_VIDEO_HID;
+ cid_add(device, ACPI_VIDEO_HID, NULL);
else if (ACPI_SUCCESS(acpi_bay_match(device)))
- cid_add = ACPI_BAY_HID;
+ cid_add(device, ACPI_BAY_HID, NULL);
else if (ACPI_SUCCESS(acpi_dock_match(device)))
- cid_add = ACPI_DOCK_HID;
+ cid_add(device, ACPI_DOCK_HID, NULL);;
break;
case ACPI_BUS_TYPE_POWER:
@@ -1038,47 +1082,6 @@ static void acpi_device_set_id(struct ac
strcpy(device->pnp.unique_id, uid);
device->flags.unique_id = 1;
}
- if (cid_list || cid_add) {
- struct acpi_compatible_id_list *list;
- int size = 0;
- int count = 0;
-
- if (cid_list) {
- size = cid_list->size;
- } else if (cid_add) {
- size = sizeof(struct acpi_compatible_id_list);
- cid_list = ACPI_ALLOCATE_ZEROED((acpi_size) size);
- if (!cid_list) {
- printk(KERN_ERR "Memory allocation error\n");
- kfree(buffer.pointer);
- return;
- } else {
- cid_list->count = 0;
- cid_list->size = size;
- }
- }
- if (cid_add)
- size += sizeof(struct acpi_compatible_id);
- list = kmalloc(size, GFP_KERNEL);
-
- if (list) {
- if (cid_list) {
- memcpy(list, cid_list, cid_list->size);
- count = cid_list->count;
- }
- if (cid_add) {
- strncpy(list->id[count].value, cid_add,
- ACPI_MAX_CID_LENGTH);
- count++;
- device->flags.compatible_ids = 1;
- }
- list->size = size;
- list->count = count;
- device->pnp.cid_list = list;
- } else
- printk(KERN_ERR PREFIX "Memory allocation error\n");
- }
-
kfree(buffer.pointer);
}
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 2/7] ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device 2008-04-16 18:52 [PATCH 2/7] ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device Thomas Renninger @ 2008-04-17 9:02 ` Zhao Yakui 2008-05-13 18:30 ` [PATCH 2/7] ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device - snd version Thomas Renninger 0 siblings, 1 reply; 3+ messages in thread From: Zhao Yakui @ 2008-04-17 9:02 UTC (permalink / raw) To: trenn Cc: Len Brown, linux-acpi, Jonathan Woithe, Carlos Corbacho, Corentin CHARY, Henrique de Moraes Holschuh, Mattia Dongili On Wed, 2008-04-16 at 20:52 +0200, Thomas Renninger wrote: > ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device > > Rip the functionality out of acpi_device_set_id, it is needed elsewhere in > the code on follow-up patches. > It seems that the cid_add function will be called twice for some devices. For example: VIDEO device When the CID object exists, the cid_add function will be called to add the compatible_list. When it is identified as the VIDEO device, the cid_add function will be called again to add the compatible_list. In such case the compatible_list pointer will be replaced directly by new pointer and the memory pointed by compatible_list can't be free. Best regards. Yakui > Signed-off-by: Thomas Renninger <trenn@suse.de> > > --- > drivers/acpi/scan.c | 97 ++++++++++++++++++++++++++-------------------------- > 1 file changed, 50 insertions(+), 47 deletions(-) > > Index: linux-acpi-2.6_video_native_vs_vendor/drivers/acpi/scan.c > =================================================================== > --- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/acpi/scan.c > +++ linux-acpi-2.6_video_native_vs_vendor/drivers/acpi/scan.c > @@ -954,6 +954,52 @@ static int acpi_dock_match(struct acpi_d > return acpi_get_handle(device->handle, "_DCK", &tmp); > } > > +/* Add a single Compatible ID or a whole list of CIDs to a device */ > +static void cid_add(struct acpi_device *device, const char *cid_add, > + struct acpi_compatible_id_list *cid_list) { > + > + struct acpi_compatible_id_list *list = NULL; > + int size = 0; > + int count = 0; > + > + if (!cid_list && !cid_add) > + return; > + > + if (cid_list) { > + size = cid_list->size; > + } else if (cid_add) { > + size = sizeof(struct acpi_compatible_id_list); > + cid_list = ACPI_ALLOCATE_ZEROED((acpi_size) size); > + if (!cid_list) { > + printk(KERN_ERR "Memory allocation error\n"); > + return; > + } else { > + cid_list->count = 0; > + cid_list->size = size; > + } > + } > + if (cid_add) > + size += sizeof(struct acpi_compatible_id); > + list = kmalloc(size, GFP_KERNEL); > + > + if (list) { > + if (cid_list) { > + memcpy(list, cid_list, cid_list->size); > + count = cid_list->count; > + } > + if (cid_add) { > + strncpy(list->id[count].value, cid_add, > + ACPI_MAX_CID_LENGTH); > + count++; > + device->flags.compatible_ids = 1; > + } > + list->size = size; > + list->count = count; > + device->pnp.cid_list = list; > + } else > + printk(KERN_ERR PREFIX "Memory allocation error\n"); > +} > + > static void acpi_device_set_id(struct acpi_device *device, > struct acpi_device *parent, acpi_handle handle, > int type) > @@ -962,8 +1008,6 @@ static void acpi_device_set_id(struct ac > struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; > char *hid = NULL; > char *uid = NULL; > - struct acpi_compatible_id_list *cid_list = NULL; > - const char *cid_add = NULL; > acpi_status status; > > switch (type) { > @@ -980,7 +1024,7 @@ static void acpi_device_set_id(struct ac > if (info->valid & ACPI_VALID_UID) > uid = info->unique_id.value; > if (info->valid & ACPI_VALID_CID) > - cid_list = &info->compatibility_id; > + cid_add(device, NULL, &info->compatibility_id); > if (info->valid & ACPI_VALID_ADR) { > device->pnp.bus_address = info->address; > device->flags.bus_address = 1; > @@ -992,11 +1036,11 @@ static void acpi_device_set_id(struct ac > against another driver. > */ > if (ACPI_SUCCESS(acpi_video_bus_match(device))) > - cid_add = ACPI_VIDEO_HID; > + cid_add(device, ACPI_VIDEO_HID, NULL); > else if (ACPI_SUCCESS(acpi_bay_match(device))) > - cid_add = ACPI_BAY_HID; > + cid_add(device, ACPI_BAY_HID, NULL); > else if (ACPI_SUCCESS(acpi_dock_match(device))) > - cid_add = ACPI_DOCK_HID; > + cid_add(device, ACPI_DOCK_HID, NULL);; > > break; > case ACPI_BUS_TYPE_POWER: > @@ -1038,47 +1082,6 @@ static void acpi_device_set_id(struct ac > strcpy(device->pnp.unique_id, uid); > device->flags.unique_id = 1; > } > - if (cid_list || cid_add) { > - struct acpi_compatible_id_list *list; > - int size = 0; > - int count = 0; > - > - if (cid_list) { > - size = cid_list->size; > - } else if (cid_add) { > - size = sizeof(struct acpi_compatible_id_list); > - cid_list = ACPI_ALLOCATE_ZEROED((acpi_size) size); > - if (!cid_list) { > - printk(KERN_ERR "Memory allocation error\n"); > - kfree(buffer.pointer); > - return; > - } else { > - cid_list->count = 0; > - cid_list->size = size; > - } > - } > - if (cid_add) > - size += sizeof(struct acpi_compatible_id); > - list = kmalloc(size, GFP_KERNEL); > - > - if (list) { > - if (cid_list) { > - memcpy(list, cid_list, cid_list->size); > - count = cid_list->count; > - } > - if (cid_add) { > - strncpy(list->id[count].value, cid_add, > - ACPI_MAX_CID_LENGTH); > - count++; > - device->flags.compatible_ids = 1; > - } > - list->size = size; > - list->count = count; > - device->pnp.cid_list = list; > - } else > - printk(KERN_ERR PREFIX "Memory allocation error\n"); > - } > - > kfree(buffer.pointer); > } > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 2/7] ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device - snd version 2008-04-17 9:02 ` Zhao Yakui @ 2008-05-13 18:30 ` Thomas Renninger 0 siblings, 0 replies; 3+ messages in thread From: Thomas Renninger @ 2008-05-13 18:30 UTC (permalink / raw) To: Zhao Yakui Cc: Len Brown, linux-acpi, Jonathan Woithe, Carlos Corbacho, Corentin CHARY, Henrique de Moraes Holschuh, Mattia Dongili Hi Yakui, On Thu, 2008-04-17 at 17:02 +0800, Zhao Yakui wrote: > On Wed, 2008-04-16 at 20:52 +0200, Thomas Renninger wrote: > > ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device > > > > Rip the functionality out of acpi_device_set_id, it is needed elsewhere in > > the code on follow-up patches. > > > It seems that the cid_add function will be called twice for some > devices. > For example: VIDEO device > When the CID object exists, the cid_add function will be called to > add the compatible_list. > When it is identified as the VIDEO device, the cid_add function will > be called again to add the compatible_list. > > In such case the compatible_list pointer will be replaced directly by > new pointer and the memory pointed by compatible_list can't be free. Can you give this another review, pls. Thanks, Thomas ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device Cleanup: Introduce a new function to add CIDs (single or in a list). Fix a memleak: cid_list must not be allocated Signed-off-by: Thomas Renninger <trenn@suse.de> --- drivers/acpi/scan.c | 99 +++++++++++++++++++++++++++------------------------- 1 file changed, 52 insertions(+), 47 deletions(-) Index: linux-acpi-2.6_video_native_vs_vendor/drivers/acpi/scan.c =================================================================== --- linux-acpi-2.6_video_native_vs_vendor.orig/drivers/acpi/scan.c +++ linux-acpi-2.6_video_native_vs_vendor/drivers/acpi/scan.c @@ -44,6 +44,12 @@ static int create_modalias(struct acpi_d if (!acpi_dev->flags.hardware_id && !acpi_dev->flags.compatible_ids) return -ENODEV; + /* This could happen if _CID is available, but for some reason, the + content has not been added */ + if (acpi_dev->flags.compatible_ids && !acpi_dev->pnp.cid_list) { + return -ENODEV; + } + len = snprintf(modalias, size, "acpi:"); size -= len; @@ -994,6 +1000,48 @@ static int acpi_dock_match(struct acpi_d return acpi_get_handle(device->handle, "_DCK", &tmp); } +/* Add a single Compatible ID or a whole list of CIDs to a device */ +static void cid_add(struct acpi_device *device, const char *cid_add, + const struct acpi_compatible_id_list *cid_list) { + + struct acpi_compatible_id_list *list = NULL; + struct acpi_compatible_id_list tmp_list; + int size = 0; + int count = 0; + + if (!cid_list && !cid_add) + return; + + if (cid_list) { + size = cid_list->size; + } else { + size = sizeof(struct acpi_compatible_id_list); + tmp_list.count = 0; + tmp_list.size = size; + } + if (cid_add) + size += sizeof(struct acpi_compatible_id); + list = kzalloc(size, GFP_KERNEL); + + if (list) { + if (cid_list) { + memcpy(list, cid_list, cid_list->size); + count = cid_list->count; + } + if (cid_add) { + strncpy(list->id[count].value, cid_add, + ACPI_MAX_CID_LENGTH); + count++; + device->flags.compatible_ids = 1; + } + list->size = size; + list->count = count; + kfree(device->pnp.cid_list); + device->pnp.cid_list = list; + } else + printk(KERN_ERR PREFIX "Memory allocation error\n"); +} + static void acpi_device_set_id(struct acpi_device *device, struct acpi_device *parent, acpi_handle handle, int type) @@ -1002,8 +1050,6 @@ static void acpi_device_set_id(struct ac struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL }; char *hid = NULL; char *uid = NULL; - struct acpi_compatible_id_list *cid_list = NULL; - const char *cid_add = NULL; acpi_status status; switch (type) { @@ -1020,7 +1066,7 @@ static void acpi_device_set_id(struct ac if (info->valid & ACPI_VALID_UID) uid = info->unique_id.value; if (info->valid & ACPI_VALID_CID) - cid_list = &info->compatibility_id; + cid_add(device, NULL, &info->compatibility_id); if (info->valid & ACPI_VALID_ADR) { device->pnp.bus_address = info->address; device->flags.bus_address = 1; @@ -1032,11 +1078,11 @@ static void acpi_device_set_id(struct ac against another driver. */ if (ACPI_SUCCESS(acpi_video_bus_match(device))) - cid_add = ACPI_VIDEO_HID; + cid_add(device, ACPI_VIDEO_HID, NULL); else if (ACPI_SUCCESS(acpi_bay_match(device))) - cid_add = ACPI_BAY_HID; + cid_add(device, ACPI_BAY_HID, NULL); else if (ACPI_SUCCESS(acpi_dock_match(device))) - cid_add = ACPI_DOCK_HID; + cid_add(device, ACPI_DOCK_HID, NULL);; break; case ACPI_BUS_TYPE_POWER: @@ -1078,47 +1124,6 @@ static void acpi_device_set_id(struct ac strcpy(device->pnp.unique_id, uid); device->flags.unique_id = 1; } - if (cid_list || cid_add) { - struct acpi_compatible_id_list *list; - int size = 0; - int count = 0; - - if (cid_list) { - size = cid_list->size; - } else if (cid_add) { - size = sizeof(struct acpi_compatible_id_list); - cid_list = ACPI_ALLOCATE_ZEROED((acpi_size) size); - if (!cid_list) { - printk(KERN_ERR "Memory allocation error\n"); - kfree(buffer.pointer); - return; - } else { - cid_list->count = 0; - cid_list->size = size; - } - } - if (cid_add) - size += sizeof(struct acpi_compatible_id); - list = kmalloc(size, GFP_KERNEL); - - if (list) { - if (cid_list) { - memcpy(list, cid_list, cid_list->size); - count = cid_list->count; - } - if (cid_add) { - strncpy(list->id[count].value, cid_add, - ACPI_MAX_CID_LENGTH); - count++; - device->flags.compatible_ids = 1; - } - list->size = size; - list->count = count; - device->pnp.cid_list = list; - } else - printk(KERN_ERR PREFIX "Memory allocation error\n"); - } - kfree(buffer.pointer); } ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-05-13 16:30 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-04-16 18:52 [PATCH 2/7] ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device Thomas Renninger 2008-04-17 9:02 ` Zhao Yakui 2008-05-13 18:30 ` [PATCH 2/7] ACPI: Introduce static func: cid_add to add IDs as compatible IDs to a device - snd version Thomas Renninger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox