* [PATCH 0/2] PS3: Adjustments for six function implementations @ 2017-12-16 13:50 SF Markus Elfring 2017-12-16 13:51 ` [PATCH 1/2] ps3: Delete an error message for a failed memory allocation in two functions SF Markus Elfring ` (2 more replies) 0 siblings, 3 replies; 11+ messages in thread From: SF Markus Elfring @ 2017-12-16 13:50 UTC (permalink / raw) To: linuxppc-dev, Benjamin Herrenschmidt, Geoff Levand, Michael Ellerman, Paul Mackerras Cc: LKML, kernel-janitors From: Markus Elfring <elfring@users.sourceforge.net> Date: Sat, 16 Dec 2017 14:42:24 +0100 Two update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an error message for a failed memory allocation in two functions Improve a size determination in five functions drivers/ps3/ps3-lpm.c | 2 -- drivers/ps3/ps3-sys-manager.c | 9 ++------- drivers/ps3/ps3-vuart.c | 12 +++--------- 3 files changed, 5 insertions(+), 18 deletions(-) -- 2.15.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/2] ps3: Delete an error message for a failed memory allocation in two functions 2017-12-16 13:50 [PATCH 0/2] PS3: Adjustments for six function implementations SF Markus Elfring @ 2017-12-16 13:51 ` SF Markus Elfring 2017-12-18 18:22 ` Geoff Levand 2017-12-16 13:54 ` [PATCH 2/2] ps3: Improve a size determination in five functions SF Markus Elfring 2017-12-18 18:32 ` [PATCH 0/2] PS3: Adjustments for six function implementations Geoff Levand 2 siblings, 1 reply; 11+ messages in thread From: SF Markus Elfring @ 2017-12-16 13:51 UTC (permalink / raw) To: linuxppc-dev, Benjamin Herrenschmidt, Geoff Levand, Michael Ellerman, Paul Mackerras Cc: LKML, kernel-janitors From: Markus Elfring <elfring@users.sourceforge.net> Date: Sat, 16 Dec 2017 12:32:42 +0100 Omit an extra message for a memory allocation failure in these functions. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> --- drivers/ps3/ps3-lpm.c | 2 -- drivers/ps3/ps3-vuart.c | 1 - 2 files changed, 3 deletions(-) diff --git a/drivers/ps3/ps3-lpm.c b/drivers/ps3/ps3-lpm.c index e34de9a7d517..ac8d5725c9b4 100644 --- a/drivers/ps3/ps3-lpm.c +++ b/drivers/ps3/ps3-lpm.c @@ -1123,8 +1123,6 @@ int ps3_lpm_open(enum ps3_lpm_tb_type tb_type, void *tb_cache, lpm_priv->tb_cache_internal = kzalloc( lpm_priv->tb_cache_size + 127, GFP_KERNEL); if (!lpm_priv->tb_cache_internal) { - dev_err(sbd_core(), "%s:%u: alloc internal tb_cache " - "failed\n", __func__, __LINE__); result = -ENOMEM; goto fail_malloc; } diff --git a/drivers/ps3/ps3-vuart.c b/drivers/ps3/ps3-vuart.c index b7f300b79ffd..bbaad30a876f 100644 --- a/drivers/ps3/ps3-vuart.c +++ b/drivers/ps3/ps3-vuart.c @@ -929,7 +929,6 @@ static int ps3_vuart_bus_interrupt_get(void) vuart_bus_priv.bmp = kzalloc(sizeof(struct ports_bmp), GFP_KERNEL); if (!vuart_bus_priv.bmp) { - pr_debug("%s:%d: kzalloc failed.\n", __func__, __LINE__); result = -ENOMEM; goto fail_bmp_malloc; } -- 2.15.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] ps3: Delete an error message for a failed memory allocation in two functions 2017-12-16 13:51 ` [PATCH 1/2] ps3: Delete an error message for a failed memory allocation in two functions SF Markus Elfring @ 2017-12-18 18:22 ` Geoff Levand 0 siblings, 0 replies; 11+ messages in thread From: Geoff Levand @ 2017-12-18 18:22 UTC (permalink / raw) To: SF Markus Elfring, linuxppc-dev, Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras Cc: LKML, kernel-janitors On 12/16/2017 05:51 AM, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Sat, 16 Dec 2017 12:32:42 +0100 > > Omit an extra message for a memory allocation failure in these functions. This is OK, I'll add it to my ps3-queue branch. -Geoff ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/2] ps3: Improve a size determination in five functions 2017-12-16 13:50 [PATCH 0/2] PS3: Adjustments for six function implementations SF Markus Elfring 2017-12-16 13:51 ` [PATCH 1/2] ps3: Delete an error message for a failed memory allocation in two functions SF Markus Elfring @ 2017-12-16 13:54 ` SF Markus Elfring 2017-12-18 18:22 ` Geoff Levand 2017-12-20 19:58 ` [PATCH 2/2] " Geoff Levand 2017-12-18 18:32 ` [PATCH 0/2] PS3: Adjustments for six function implementations Geoff Levand 2 siblings, 2 replies; 11+ messages in thread From: SF Markus Elfring @ 2017-12-16 13:54 UTC (permalink / raw) To: linuxppc-dev, Benjamin Herrenschmidt, Geoff Levand, Michael Ellerman, Paul Mackerras Cc: LKML, kernel-janitors From: Markus Elfring <elfring@users.sourceforge.net> Date: Sat, 16 Dec 2017 14:21:04 +0100 Replace the specification of data structures by variable references as the parameter for the operator "sizeof" to make the corresponding size determination a bit safer according to the Linux coding style convention. Signed-off-by: Markus Elfring <elfring@users.sourceforge.net> --- drivers/ps3/ps3-sys-manager.c | 9 ++------- drivers/ps3/ps3-vuart.c | 11 +++-------- 2 files changed, 5 insertions(+), 15 deletions(-) diff --git a/drivers/ps3/ps3-sys-manager.c b/drivers/ps3/ps3-sys-manager.c index 73e496a72113..e7d8ef93576a 100644 --- a/drivers/ps3/ps3-sys-manager.c +++ b/drivers/ps3/ps3-sys-manager.c @@ -249,9 +249,7 @@ static int ps3_sys_manager_write(struct ps3_system_bus_device *dev, BUG_ON(header->payload_size != 8 && header->payload_size != 16); BUG_ON(header->service_id > 8); - result = ps3_vuart_write(dev, header, - sizeof(struct ps3_sys_manager_header)); - + result = ps3_vuart_write(dev, header, sizeof(*header)); if (!result) result = ps3_vuart_write(dev, payload, header->payload_size); @@ -537,11 +535,8 @@ static int ps3_sys_manager_handle_cmd(struct ps3_system_bus_device *dev) static int ps3_sys_manager_handle_msg(struct ps3_system_bus_device *dev) { - int result; struct ps3_sys_manager_header header; - - result = ps3_vuart_read(dev, &header, - sizeof(struct ps3_sys_manager_header)); + int result = ps3_vuart_read(dev, &header, sizeof(header)); if (result) return result; diff --git a/drivers/ps3/ps3-vuart.c b/drivers/ps3/ps3-vuart.c index bbaad30a876f..c82362cbaf4a 100644 --- a/drivers/ps3/ps3-vuart.c +++ b/drivers/ps3/ps3-vuart.c @@ -522,8 +522,7 @@ int ps3_vuart_write(struct ps3_system_bus_device *dev, const void *buf, } else spin_unlock_irqrestore(&priv->tx_list.lock, flags); - lb = kmalloc(sizeof(struct list_buffer) + bytes, GFP_KERNEL); - + lb = kmalloc(sizeof(*lb) + bytes, GFP_KERNEL); if (!lb) return -ENOMEM; @@ -575,9 +574,7 @@ static int ps3_vuart_queue_rx_bytes(struct ps3_system_bus_device *dev, /* Add some extra space for recently arrived data. */ bytes += 128; - - lb = kmalloc(sizeof(struct list_buffer) + bytes, GFP_ATOMIC); - + lb = kmalloc(sizeof(*lb) + bytes, GFP_ATOMIC); if (!lb) return -ENOMEM; @@ -925,9 +922,7 @@ static int ps3_vuart_bus_interrupt_get(void) return 0; BUG_ON(vuart_bus_priv.bmp); - - vuart_bus_priv.bmp = kzalloc(sizeof(struct ports_bmp), GFP_KERNEL); - + vuart_bus_priv.bmp = kzalloc(sizeof(*vuart_bus_priv.bmp), GFP_KERNEL); if (!vuart_bus_priv.bmp) { result = -ENOMEM; goto fail_bmp_malloc; -- 2.15.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ps3: Improve a size determination in five functions 2017-12-16 13:54 ` [PATCH 2/2] ps3: Improve a size determination in five functions SF Markus Elfring @ 2017-12-18 18:22 ` Geoff Levand 2017-12-18 20:45 ` SF Markus Elfring 2017-12-20 19:58 ` [PATCH 2/2] " Geoff Levand 1 sibling, 1 reply; 11+ messages in thread From: Geoff Levand @ 2017-12-18 18:22 UTC (permalink / raw) To: SF Markus Elfring, linuxppc-dev, Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras Cc: LKML, kernel-janitors Hi, On 12/16/2017 05:54 AM, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Sat, 16 Dec 2017 14:21:04 +0100 > > Replace the specification of data structures by variable references > as the parameter for the operator "sizeof" to make the corresponding size > determination a bit safer according to the Linux coding style convention. This would be OK, but you are also removing empty lines and changing the coding format. Please update the patch to only make the sizeof changes. -Geoff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ps3: Improve a size determination in five functions 2017-12-18 18:22 ` Geoff Levand @ 2017-12-18 20:45 ` SF Markus Elfring 2017-12-18 20:52 ` Geoff Levand 0 siblings, 1 reply; 11+ messages in thread From: SF Markus Elfring @ 2017-12-18 20:45 UTC (permalink / raw) To: Geoff Levand, linuxppc-dev Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, LKML, kernel-janitors >> Replace the specification of data structures by variable references >> as the parameter for the operator "sizeof" to make the corresponding size >> determination a bit safer according to the Linux coding style convention. > > This would be OK, Thanks. > but you are also removing empty lines and changing the coding format. * Why do you not like such a change combination? * Do you really want to integrate further update steps in this case? Regards, Markus ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ps3: Improve a size determination in five functions 2017-12-18 20:45 ` SF Markus Elfring @ 2017-12-18 20:52 ` Geoff Levand 0 siblings, 0 replies; 11+ messages in thread From: Geoff Levand @ 2017-12-18 20:52 UTC (permalink / raw) To: SF Markus Elfring, linuxppc-dev Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, LKML, kernel-janitors On 12/18/2017 12:45 PM, SF Markus Elfring wrote: >> but you are also removing empty lines and changing the coding format. > > * Why do you not like such a change combination? > > * Do you really want to integrate further update steps in this case? I want to keep the coding style consistent throughout the file. -Geoff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] ps3: Improve a size determination in five functions 2017-12-16 13:54 ` [PATCH 2/2] ps3: Improve a size determination in five functions SF Markus Elfring 2017-12-18 18:22 ` Geoff Levand @ 2017-12-20 19:58 ` Geoff Levand 2017-12-20 21:20 ` SF Markus Elfring 1 sibling, 1 reply; 11+ messages in thread From: Geoff Levand @ 2017-12-20 19:58 UTC (permalink / raw) To: SF Markus Elfring, linuxppc-dev, Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras Cc: LKML, kernel-janitors Hi, On 12/16/2017 05:54 AM, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Sat, 16 Dec 2017 14:21:04 +0100 > > Replace the specification of data structures by variable references > as the parameter for the operator "sizeof" to make the corresponding size > determination a bit safer according to the Linux coding style convention. After some thought, I've decided to reject this patch and others like it because I feel it will make long term maintenance of the PS3 code more difficult. Some observations: o Your patch fixes no bug nor replaces any depreciated feature. o There will be no functional change; the generated binary will be nearly identical. o The PS3 kernel support is now over 10 years old. o I need to continue support for a few old kernel versions, specifically linux-3.15 and linux-2.6.30. That includes keeping them working with new toolchain versions. I need to back port fixes to these old kernels. o When problems arise I sometimes need to use git bisect back to old kernel versions. When I do the bisect I often have fixes and local debug patches that I apply to the bisected tree before building. o Source code changes between versions causes patch conflicts that need to be manually resolved. This can be error prone and very time consuming on a long bisect session. My decision to reject this patch and others like it is in attempt to minimize the code maintenance effort. If you have patches that fix bugs, upgrade depreciated features, or generally improve functionality please submit them for review. -Geoff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ps3: Improve a size determination in five functions 2017-12-20 19:58 ` [PATCH 2/2] " Geoff Levand @ 2017-12-20 21:20 ` SF Markus Elfring 2017-12-21 0:05 ` Geoff Levand 0 siblings, 1 reply; 11+ messages in thread From: SF Markus Elfring @ 2017-12-20 21:20 UTC (permalink / raw) To: Geoff Levand, linuxppc-dev Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, LKML, kernel-janitors > Some observations: > > o Your patch fixes no bug nor replaces any depreciated feature. How do you think about information from the section “14) Allocating memory” in the document “coding-style.rst” for the shown source code transformation? > o There will be no functional change; … Yes. - The suggested adjustment should work in this way generally. Regards, Markus ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ps3: Improve a size determination in five functions 2017-12-20 21:20 ` SF Markus Elfring @ 2017-12-21 0:05 ` Geoff Levand 0 siblings, 0 replies; 11+ messages in thread From: Geoff Levand @ 2017-12-21 0:05 UTC (permalink / raw) To: SF Markus Elfring, linuxppc-dev Cc: Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras, LKML, kernel-janitors On 12/20/2017 01:20 PM, SF Markus Elfring wrote: >> o Your patch fixes no bug nor replaces any depreciated feature. > > How do you think about information from the section “14) Allocating memory” > in the document “coding-style.rst” for the shown source code transformation? In terms of importance, I would put maintenance and user support as more important than coding style. Regarding Section 14 of coding-style.rst specifically, as I mentioned the PS3 support is over 10 years old. I don't expect a change to the type of any structures. If there are type changes, then we can update the allocation size parameters at that time. -Geoff ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/2] PS3: Adjustments for six function implementations 2017-12-16 13:50 [PATCH 0/2] PS3: Adjustments for six function implementations SF Markus Elfring 2017-12-16 13:51 ` [PATCH 1/2] ps3: Delete an error message for a failed memory allocation in two functions SF Markus Elfring 2017-12-16 13:54 ` [PATCH 2/2] ps3: Improve a size determination in five functions SF Markus Elfring @ 2017-12-18 18:32 ` Geoff Levand 2 siblings, 0 replies; 11+ messages in thread From: Geoff Levand @ 2017-12-18 18:32 UTC (permalink / raw) To: SF Markus Elfring, linuxppc-dev, Benjamin Herrenschmidt, Michael Ellerman, Paul Mackerras Cc: LKML, kernel-janitors Hi, On 12/16/2017 05:50 AM, SF Markus Elfring wrote: > Markus Elfring (2): > Delete an error message for a failed memory allocation in two functions > Improve a size determination in five functions As I mentioned before, please keep your commit message subjects to 50 chars or less. -Geoff ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2017-12-21 0:05 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-16 13:50 [PATCH 0/2] PS3: Adjustments for six function implementations SF Markus Elfring 2017-12-16 13:51 ` [PATCH 1/2] ps3: Delete an error message for a failed memory allocation in two functions SF Markus Elfring 2017-12-18 18:22 ` Geoff Levand 2017-12-16 13:54 ` [PATCH 2/2] ps3: Improve a size determination in five functions SF Markus Elfring 2017-12-18 18:22 ` Geoff Levand 2017-12-18 20:45 ` SF Markus Elfring 2017-12-18 20:52 ` Geoff Levand 2017-12-20 19:58 ` [PATCH 2/2] " Geoff Levand 2017-12-20 21:20 ` SF Markus Elfring 2017-12-21 0:05 ` Geoff Levand 2017-12-18 18:32 ` [PATCH 0/2] PS3: Adjustments for six function implementations Geoff Levand
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).