Skip to contentSkip to conversation

staging: gasket: interrupt: use sizeof(*p) for memory allocation

PATCH Superseded Kimberly Brown 3 participants 4 messages Oct 24, 2018 – Oct 28, 2018 Staging

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 staging: gasket: interrupt: use sizeof(*p) for memory allocation

Use sizeof(*p) instead of sizeof(struct P) for memory allocation. This change complies with the Linux kernel coding style. It improves readability and decreases the opportunity for bugs if the pointer variable type is changed. Issue found by checkpatch.

---
 drivers/staging/gasket/gasket_interrupt.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/gasket/gasket_interrupt.c +1 -2
index 49d47afad64f..8353ab50b3ec 100644
@@ -322,8 +322,7 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
 	const struct gasket_driver_desc *driver_desc =
 		gasket_get_driver_desc(gasket_dev);
 
-	interrupt_data = kzalloc(sizeof(struct gasket_interrupt_data),
-				 GFP_KERNEL);
+	interrupt_data = kzalloc(sizeof(*interrupt_data), GFP_KERNEL);
 	if (!interrupt_data)
 		return -ENOMEM;
 	gasket_dev->interrupt_data = interrupt_data;
Signature · 1 line
2.17.1
#1 lore raw
Quoted 15 lines · Kimberly Brown

Use sizeof(*p) instead of sizeof(struct P) for memory allocation. This change complies with the Linux kernel coding style. It improves readability and decreases the opportunity for bugs if the pointer variable type is changed. Issue found by checkpatch.

---
drivers/staging/gasket/gasket_interrupt.c | 3 +--

1 file changed, 1 insertion(+), 2 deletions(-)
drivers/staging/gasket/gasket_interrupt.c +0 -0
index 49d47afad64f..8353ab50b3ec 100644

drivers/staging/gasket/gasket_interrupt.c int gasket_interrupt_init(struct gasket_dev *gasket_dev) · line 328

Quoted 9 lines
drivers/staging/gasket/gasket_interrupt.c +1 -2
@@ -322,8 +322,7 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
 	const struct gasket_driver_desc *driver_desc =
 		gasket_get_driver_desc(gasket_dev);
 
-	interrupt_data = kzalloc(sizeof(struct gasket_interrupt_data),
-				 GFP_KERNEL);
+	interrupt_data = kzalloc(sizeof(*interrupt_data), GFP_KERNEL);
 	if (!interrupt_data)
 		return -ENOMEM;
 	gasket_dev->interrupt_data = interrupt_data;

This looks great. Maybe you'd like to do the same for the rest of the function as well?

--
Thanks,
Sasha

#2 lore raw

PATCH v2 1/1 staging: gasket: use sizeof(*p) for memory allocation

In reply to Kimberly Brown

Use sizeof(*p) instead of sizeof(struct P) for memory allocation. This change complies with the Linux kernel coding style. It improves readability and decreases the opportunity for bugs if the pointer variable type is changed. Issue found by checkpatch.

---
Changes in v2:
- Changed 4 additional memory allocations to use sizeof(*p).
- Removed 'interrupt:' from the subject line because the gasket_page_table.c file is now included in this patch.

 drivers/staging/gasket/gasket_interrupt.c  | 17 ++++++++---------
 drivers/staging/gasket/gasket_page_table.c |  3 ++-
 2 files changed, 10 insertions(+), 10 deletions(-)
drivers/staging/gasket/gasket_interrupt.c +8 -9
index 49d47afad64f..ad5657d213f0 100644
@@ -184,7 +184,7 @@ gasket_interrupt_msix_init(struct gasket_interrupt_data *interrupt_data)
 
 	interrupt_data->msix_entries =
 		kcalloc(interrupt_data->num_interrupts,
-			sizeof(struct msix_entry), GFP_KERNEL);
+			sizeof(*interrupt_data->msix_entries), GFP_KERNEL);
 	if (!interrupt_data->msix_entries)
 		return -ENOMEM;
 
@@ -322,8 +322,7 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
 	const struct gasket_driver_desc *driver_desc =
 		gasket_get_driver_desc(gasket_dev);
 
-	interrupt_data = kzalloc(sizeof(struct gasket_interrupt_data),
-				 GFP_KERNEL);
+	interrupt_data = kzalloc(sizeof(*interrupt_data), GFP_KERNEL);
 	if (!interrupt_data)
 		return -ENOMEM;
 	gasket_dev->interrupt_data = interrupt_data;
@@ -336,17 +335,17 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
 	interrupt_data->pack_width = driver_desc->interrupt_pack_width;
 	interrupt_data->num_configured = 0;
 
-	interrupt_data->eventfd_ctxs = kcalloc(driver_desc->num_interrupts,
-					       sizeof(struct eventfd_ctx *),
-					       GFP_KERNEL);
+	interrupt_data->eventfd_ctxs =
+		kcalloc(driver_desc->num_interrupts,
+			sizeof(*interrupt_data->eventfd_ctxs), GFP_KERNEL);
 	if (!interrupt_data->eventfd_ctxs) {
 		kfree(interrupt_data);
 		return -ENOMEM;
 	}
 
-	interrupt_data->interrupt_counts = kcalloc(driver_desc->num_interrupts,
-						   sizeof(ulong),
-						   GFP_KERNEL);
+	interrupt_data->interrupt_counts =
+		kcalloc(driver_desc->num_interrupts,
+			sizeof(*interrupt_data->interrupt_counts), GFP_KERNEL);
 	if (!interrupt_data->interrupt_counts) {
 		kfree(interrupt_data->eventfd_ctxs);
 		kfree(interrupt_data);
drivers/staging/gasket/gasket_page_table.c +2 -1
index 5b398b7ba81d..5f4c9b49cb5a 100644
@@ -1302,7 +1302,8 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
 
 	/* allocate the physical memory block */
 	gasket_dev->page_table[index]->coherent_pages =
-		kcalloc(num_pages, sizeof(struct gasket_coherent_page_entry),
+		kcalloc(num_pages,
+			sizeof(*gasket_dev->page_table[index]->coherent_pages),
 			GFP_KERNEL);
 	if (!gasket_dev->page_table[index]->coherent_pages)
 		goto nomem;
Signature · 1 line
2.17.1
Acked-by on v2 1/1
#3 lore raw

Re: [PATCH v2] staging: gasket: use sizeof(*p) for memory allocation

Quoted 79 lines · Kimberly Brown

Use sizeof(*p) instead of sizeof(struct P) for memory allocation. This change complies with the Linux kernel coding style. It improves readability and decreases the opportunity for bugs if the pointer variable type is changed. Issue found by checkpatch.

---
Changes in v2:
- Changed 4 additional memory allocations to use sizeof(*p).
- Removed 'interrupt:' from the subject line because the gasket_page_table.c file is now included in this patch.

 drivers/staging/gasket/gasket_interrupt.c  | 17 ++++++++---------
 drivers/staging/gasket/gasket_page_table.c |  3 ++-
 2 files changed, 10 insertions(+), 10 deletions(-)
drivers/staging/gasket/gasket_interrupt.c +8 -9
index 49d47afad64f..ad5657d213f0 100644
@@ -184,7 +184,7 @@ gasket_interrupt_msix_init(struct gasket_interrupt_data *interrupt_data)
 
        interrupt_data->msix_entries =
                kcalloc(interrupt_data->num_interrupts,
-                       sizeof(struct msix_entry), GFP_KERNEL);
+                       sizeof(*interrupt_data->msix_entries), GFP_KERNEL);
        if (!interrupt_data->msix_entries)
                return -ENOMEM;
 
@@ -322,8 +322,7 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
        const struct gasket_driver_desc *driver_desc =
                gasket_get_driver_desc(gasket_dev);
 
-       interrupt_data = kzalloc(sizeof(struct gasket_interrupt_data),
-                                GFP_KERNEL);
+       interrupt_data = kzalloc(sizeof(*interrupt_data), GFP_KERNEL);
        if (!interrupt_data)
                return -ENOMEM;
        gasket_dev->interrupt_data = interrupt_data;
@@ -336,17 +335,17 @@ int gasket_interrupt_init(struct gasket_dev *gasket_dev)
        interrupt_data->pack_width = driver_desc->interrupt_pack_width;
        interrupt_data->num_configured = 0;
 
-       interrupt_data->eventfd_ctxs = kcalloc(driver_desc->num_interrupts,
-                                              sizeof(struct eventfd_ctx *),
-                                              GFP_KERNEL);
+       interrupt_data->eventfd_ctxs =
+               kcalloc(driver_desc->num_interrupts,
+                       sizeof(*interrupt_data->eventfd_ctxs), GFP_KERNEL);
        if (!interrupt_data->eventfd_ctxs) {
                kfree(interrupt_data);
                return -ENOMEM;
        }
 
-       interrupt_data->interrupt_counts = kcalloc(driver_desc->num_interrupts,
-                                                  sizeof(ulong),
-                                                  GFP_KERNEL);
+       interrupt_data->interrupt_counts =
+               kcalloc(driver_desc->num_interrupts,
+                       sizeof(*interrupt_data->interrupt_counts), GFP_KERNEL);
        if (!interrupt_data->interrupt_counts) {
                kfree(interrupt_data->eventfd_ctxs);
                kfree(interrupt_data);
drivers/staging/gasket/gasket_page_table.c +2 -2
index 5b398b7ba81d..5f4c9b49cb5a 100644
@@ -1302,7 +1302,8 @@ int gasket_alloc_coherent_memory(struct gasket_dev *gasket_dev, u64 size,
 
        /* allocate the physical memory block */
        gasket_dev->page_table[index]->coherent_pages =
-               kcalloc(num_pages, sizeof(struct gasket_coherent_page_entry),
+               kcalloc(num_pages,
+                       sizeof(*gasket_dev->page_table[index]->coherent_pages),
                        GFP_KERNEL);
        if (!gasket_dev->page_table[index]->coherent_pages)
                goto nomem;
--
Quoted 1 line

2.17.1

Thanks!