[PATCH 3/3] mtd-rfd_ftl: Refactoring for erase_block()

2015-12-31 Thread SF Markus Elfring
From: Markus Elfring Date: Thu, 31 Dec 2015 21:06:27 +0100 This issue was detected by using the Coccinelle software. * Return directly if a memory allocation failed. * Drop the explicit initialisation for the variable "rc" at the beginning then. Signed-off-by:

[PATCH 1/3] mtd-rfd_ftl: Replace a variable initialisation by assignments in move_block_contents()

2015-12-31 Thread SF Markus Elfring
From: Markus Elfring Date: Thu, 31 Dec 2015 20:34:51 +0100 Replace an explicit initialisation for the variable "rc" at the beginning by assignments that will only be performed if a memory allocation failed. Signed-off-by: Markus Elfring

[PATCH 0/3] net-gianfar: Fine-tuning for gfar_ethflow_to_filer_table()

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 13:15:34 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (3): Less function calls after error detection Delete unnecessary variable initialisations Extend an

[PATCH] net-huawei_cdc_ncm: Delete an unnecessary variable initialisation in huawei_cdc_ncm_bind()

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 16:54:13 +0100 Omit explicit initialisation at the beginning for one local variable that is redefined before its first use. Signed-off-by: Markus Elfring ---

[PATCH 1/3] net-gianfar: Less function calls in gfar_ethflow_to_filer_table() after error detection

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 11:16:04 +0100 The kfree() function was called in one case by the gfar_ethflow_to_filer_table() function during error handling even if a passed variable contained a null pointer. * Return directly if a memory allocation

[PATCH 2/3] net-gianfar: Delete unnecessary variable initialisations in gfar_ethflow_to_filer_table()

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 12:56:23 +0100 Omit explicit initialisation at the beginning for four local variables which are redefined before their first use. Signed-off-by: Markus Elfring ---

Re: [PATCH 1/3] net-gianfar: Less function calls in gfar_ethflow_to_filer_table() after error detection

2016-01-01 Thread SF Markus Elfring
>> +++ b/drivers/net/ethernet/freescale/gianfar_ethtool.c >> @@ -778,11 +778,13 @@ static int gfar_ethflow_to_filer_table(struct >> gfar_private *priv, u64 ethflow, >> >> local_rqfpr = kmalloc_array(MAX_FILER_IDX + 1, sizeof(unsigned int), >> GFP_KERNEL);

[PATCH] net-i40e: Replace variable initialisations by assignments in i40e_vc_get_vf_resources_msg()

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 15:11:09 +0100 Replace explicit initialisations for four local variables at the beginning by assignments that will only be performed if the corresponding code will really be executed. Signed-off-by: Markus Elfring

[PATCH 3/3] net-gianfar: Extend an initialisation clause of a for loop in gfar_ethflow_to_filer_table()

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 13:00:06 +0100 Move the assignment for the variable "j" from the beginning into an initialisation clause of a for loop. Signed-off-by: Markus Elfring ---

[PATCH v2 1/3] net-gianfar: Less function calls in gfar_ethflow_to_filer_table() after error detection

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 13:56:09 +0100 The kfree() function was called in one case by the gfar_ethflow_to_filer_table() function during error handling even if a passed variable contained a null pointer. * Return directly if a memory allocation

[PATCH] net-brcmfmac: Delete an unnecessary variable initialisation in brcmf_sdio_download_firmware()

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 20:20:15 +0100 Omit explicit initialisation at the beginning for one local variable that is redefined before its first use. Signed-off-by: Markus Elfring ---

Re: [PATCH] staging-slicoss: Replace variable initialisations by assignments in slic_if_init()

2016-01-03 Thread SF Markus Elfring
>> I prefer that assignments for variables like "card" and "slic_regs" >> will only be performed immediately before the corresponding content will be >> read again (after a few condition checks were executed). >> >> Another description could be this view: >> I suggest to move the variable

Re: [PATCH] staging-slicoss: Replace variable initialisations by assignments in slic_if_init()

2016-01-03 Thread SF Markus Elfring
>> I am a bit surprised that you do not like such source code fine-tuning. > > It's moving stuff around for no real reason, why would I like it? Can such fine-tuning result in positive effects for the run-time behaviour? > Reading and reviewing and applying this type of stuff takes away from >

Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_interrupt()

2016-01-03 Thread SF Markus Elfring
>> Pass the address of the data structure element "time" directly in calls >> of the function "rtc_update_irq" instead of an extra initialisation >> for one local variable at the beginning. > > Why is it better? I suggest to refer to the data item "rtc_data->rtc" directly because the variable

Re: [PATCH 7/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_interrupt()

2016-01-03 Thread SF Markus Elfring
>> Pass the address of the data structure element "time" directly in calls >> of the function "rtc_update_irq" instead of an extra initialisation >> for one local variable at the beginning. > > Also, I don't see anything related to time in this patch. I should have referred to the data structure

Re: [PATCH] staging-slicoss: Replace variable initialisations by assignments in slic_if_init()

2016-01-03 Thread SF Markus Elfring
>> Replace explicit initialisation for two local variables at the beginning >> by assignments. > > Why? I prefer that assignments for variables like "card" and "slic_regs" will only be performed immediately before the corresponding content will be read again (after a few condition checks were

Re: [PATCH 8/8] rtc-ab-b5ze-s3: Delete an unnecessary variable in _abb5zes3_rtc_set_timer()

2016-01-03 Thread SF Markus Elfring
>> ret = regmap_update_bits(data->regmap, ABB5ZES3_REG_TIM_CLK, >> - mask, ABB5ZES3_REG_TIM_CLK_TAC1); >> + ABB5ZES3_REG_TIM_CLK_TAC0 >> + | ABB5ZES3_REG_TIM_CLK_TAC1, >> +

[PATCH 1/2] net-qmi_wwan: Refactoring for qmi_wwan_bind()

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 17:32:07 +0100 Reduce the scope for the local variable "desc" to one branch of an if statement. Signed-off-by: Markus Elfring --- drivers/net/usb/qmi_wwan.c | 3 ++- 1 file changed, 2

[PATCH 0/2] net-ath9k_htc: Fine-tuning for two function implementations

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 19:16:05 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an unnecessary variable initialisation in ath9k_hif_usb_rx_stream() Replace a variable

[PATCH 0/2] net-qmi_wwan: Fine-tuning for two function implementations

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 17:47:46 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (2): Refactoring for qmi_wwan_bind() Delete an unnecessary variable initialisation in

[PATCH 2/2] net-qmi_wwan: Delete an unnecessary variable initialisation in qmi_wwan_register_subdriver()

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 17:35:03 +0100 Omit explicit initialisation at the beginning for one local variable that is redefined before its first use. Signed-off-by: Markus Elfring --- drivers/net/usb/qmi_wwan.c | 2

[PATCH 2/2] net-ath9k_htc: Replace a variable initialisation by an assignment in ath9k_htc_set_channel()

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 19:09:32 +0100 Replace an explicit initialisation for one local variable at the beginning by a conditional assignment. Signed-off-by: Markus Elfring ---

[PATCH 1/2] net-ath9k_htc: Delete an unnecessary variable initialisation in ath9k_hif_usb_rx_stream()

2016-01-01 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 1 Jan 2016 19:00:53 +0100 Omit explicit initialisation at the beginning for one local variable that is redefined before its first use. Signed-off-by: Markus Elfring ---

Re: staging-slicoss: Replace variable initialisations by assignments in slic_if_init()

2016-01-03 Thread SF Markus Elfring
>> Can such fine-tuning result in positive effects for the run-time behaviour? > > If you can not benchmark and show the proof, don't even start to claim > such a thing. Which measurement results would you accept for further discussion? >> My suggestions can result in measurable differences. >

[PATCH v2 4/4] staging: lustre: Fix a jump label position in osc_get_info()

2015-12-21 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 21 Dec 2015 19:30:42 +0100 The script "checkpatch.pl" pointed out that labels should not be indented. Thus delete a horizontal tab before the jump label "out" in the function "osc_get_info". Signed-off-by: Markus Elfring

[PATCH v2 3/4] staging: lustre: Less checks in mgc_process_recover_log() after error detection

2015-12-21 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 21 Dec 2015 18:58:51 +0100 A few checks would be performed by the mgc_process_recover_log() function even though it was determined that the passed variable "pages" contained a null pointer or a call of the alloc_page() function

[PATCH v2 2/4] staging: lustre: Delete an unnecessary variable initialisation in mgc_process_recover_log()

2015-12-21 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 21 Dec 2015 18:24:45 +0100 The variable "mne_swab" will eventually be set to an appropriate value from a call of the ptlrpc_rep_need_swab() function. Thus let us omit the explicit initialisation at the beginning. Signed-off-by:

[PATCH 0/2] blackfin-cpufreq: Fine-tuning for properties of one function

2015-12-21 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 21 Dec 2015 22:24:18 +0100 Another update suggestion was taken into account after a patch was applied from static source code analysis. Markus Elfring (2): Change return type of cpu_set_cclk() to that of clk_set_rate() Mark

[PATCH v2 0/4] staging-Lustre: Fine-tuning for some function implementations

2015-12-21 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 21 Dec 2015 20:00:02 +0100 Several update suggestions were taken into account from static source code analysis. Markus Elfring (4): Delete unnecessary goto statements in six functions Delete an unnecessary variable

[PATCH 1/2] blackfin-cpufreq: Change return type of cpu_set_cclk() to that of clk_set_rate()

2015-12-21 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 21 Dec 2015 21:56:27 +0100 The return type "unsigned long" was used by the cpu_set_cclk() function while the type "int" is provided by the clk_set_rate() function. Let us make this usage consistent. This issue was detected by using

[PATCH 2/2] blackfin-cpufreq: Mark cpu_set_cclk() as static

2015-12-21 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 21 Dec 2015 22:12:26 +0100 The cpu_set_cclk() function was only used in a single source file so far. Indicate this setting also by the corresponding linkage specifier. Signed-off-by: Markus Elfring

[PATCH v2 1/4] staging: lustre: Delete unnecessary goto statements in six functions

2015-12-21 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 21 Dec 2015 18:15:45 +0100 Six goto statements referred to a source code position directly behind them. Thus omit such unnecessary jumps. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring

[PATCH] block-LDM: One function call less in ldm_validate_tocblocks() after error detection

2015-12-23 Thread SF Markus Elfring
From: Markus Elfring Date: Tue, 22 Dec 2015 22:32:07 +0100 This issue was detected by using the Coccinelle software. Let us return directly if a memory allocation failed. Signed-off-by: Markus Elfring --- block/partitions/ldm.c |

[PATCH v2 0/2] USB-FHCI: Use return type "int" for two functions

2015-12-22 Thread SF Markus Elfring
From: Markus Elfring Date: Tue, 22 Dec 2015 16:43:12 +0100 Another update suggestion was taken into account after a patch was applied from static source code analysis. Markus Elfring (2): Use a signed return type for fhci_create_ep() Use a signed return type

ACPI-fan: Another source code review around null pointer handling?

2015-12-25 Thread SF Markus Elfring
Hello, I have looked at the source file for the ACPI fan driver once more. I would appreciate if a specific implementation detail can be clarified there. Static source code analysis can point out that functions like the following share an approach for error detection and corresponding exception

[PATCH 0/5] block-LDM: Improvements for exception handling

2015-12-23 Thread SF Markus Elfring
From: Markus Elfring Date: Wed, 23 Dec 2015 17:32:12 +0100 Several update suggestions were taken into account from static source code analysis. Markus Elfring (5): One function call less in ldm_validate_tocblocks() after error detection Delete extra log

[PATCH 3/5] block-LDM: One function call less in ldm_partition() after error detection

2015-12-23 Thread SF Markus Elfring
From: Markus Elfring Date: Wed, 23 Dec 2015 13:32:51 +0100 Let us return directly if a memory allocation failed. Signed-off-by: Markus Elfring --- block/partitions/ldm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff

[PATCH 4/5] block-LDM: Less function calls in ldm_validate_privheads() after error detection

2015-12-23 Thread SF Markus Elfring
From: Markus Elfring Date: Wed, 23 Dec 2015 14:31:01 +0100 The kfree() function was called by the ldm_validate_privheads() function during error handling even if the passed variable "ph" contained a null pointer. * Corresponding implementation details could be

[PATCH 2/5] block-LDM: Delete extra log messages for memory allocation failures

2015-12-23 Thread SF Markus Elfring
From: Markus Elfring Date: Wed, 23 Dec 2015 13:21:01 +0100 A failed call of the kmalloc() function can generate appropriate information about insufficient memory already. Thus remove unnecessary log messages for such failures. Suggested-by: Julia Lawall

[PATCH 5/5] block-LDM: Fine-tuning for the source code formatting

2015-12-23 Thread SF Markus Elfring
From: Markus Elfring Date: Wed, 23 Dec 2015 17:23:25 +0100 1. Remove some space characters and add a few according to the Linux coding style convention. 2. Reformat a bit of source code also for one switch statement. 3. Apply a few more recommendations from

[PATCH 1/5] block-LDM: One function call less in ldm_validate_tocblocks() after error detection

2015-12-23 Thread SF Markus Elfring
From: Markus Elfring Date: Tue, 22 Dec 2015 22:32:07 +0100 This issue was detected by using the Coccinelle software. Let us return directly if a memory allocation failed. Signed-off-by: Markus Elfring --- block/partitions/ldm.c |

[PATCH] gpio-ucb1400: Delete an unnecessary variable initialisation in ucb1400_gpio_probe()

2015-12-25 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 25 Dec 2015 19:36:20 +0100 The variable "err" will eventually be set to an appropriate value from a call of the gpiochip_add() function. Thus let us omit the explicit initialisation at the beginning. Signed-off-by: Markus Elfring

sata_mv: Another source code review around exception handling?

2015-12-25 Thread SF Markus Elfring
Hello, I have looked at the source file for the Marvell SATA support driver once more. I would appreciate if a specific implementation detail can be clarified there. Static source code analysis can point out that functions like the following are called by the mv_platform_probe() function.

[PATCH] i2c-core: One function call less in acpi_i2c_space_handler() after error detection

2015-12-25 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 07:30:35 +0100 The kfree() function was called in one case by the acpi_i2c_space_handler() function during error handling even if the passed variable "client" contained a null pointer. Implementation details could be

[PATCH v2] i2c-core: One function call less in acpi_i2c_space_handler() after error detection

2015-12-25 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 08:00:52 +0100 The kfree() function was called in one case by the acpi_i2c_space_handler() function during error handling even if the passed variable "client" contained a null pointer. Implementation details could be

[PATCH 4/6] InfiniBand-ocrdma: Return a value from a function call in _ocrdma_modify_qp() directly

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 18:40:43 +0100 Return the value from a call of the ocrdma_mbx_modify_qp() function without using an extra assignment for the local variable "status". Signed-off-by: Markus Elfring ---

[PATCH 5/6] InfiniBand-ocrdma: Returning only value constants in ocrdma_resize_cq()

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 18:54:47 +0100 Return constant integer values without storing them in the local variable "status". Signed-off-by: Markus Elfring --- drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 9

[PATCH 6/6] InfiniBand-ocrdma: Delete an unnecessary variable in ocrdma_dealloc_pd()

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 19:09:23 +0100 1. Return zero in one case directly. 2. Return the value from a call of the _ocrdma_dealloc_pd() function without using an extra assignment for the local variable. 3. Remove the variable "status" in

[PATCH 2/6] InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 18:18:18 +0100 The variable "status" will be set to an appropriate value a bit later. Thus let us omit the explicit initialisation at the beginning. Signed-off-by: Markus Elfring ---

[PATCH 1/6] InfiniBand-ocrdma: One variable and jump label less in ocrdma_alloc_ucontext_pd()

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 17:16:00 +0100 This issue was detected by using the Coccinelle software. * Let us return directly if a call of the _ocrdma_alloc_pd() function failed. * Delete the jump label "err" and the local variable "status"

[PATCH 0/6] InfiniBand-ocrdma: Fine-tuning for some function implementations

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 19:30:54 +0100 Several update suggestions were taken into account from static source code analysis. Markus Elfring (6): One variable and jump label less in ocrdma_alloc_ucontext_pd() Delete unnecessary variable

[PATCH 3/6] InfiniBand-ocrdma: Returning only value constants in ocrdma_qp_state_change()

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 18:28:35 +0100 Return zero at the end without using the local variable "status". Signed-off-by: Markus Elfring --- drivers/infiniband/hw/ocrdma/ocrdma_hw.c | 3 +-- 1 file changed, 1

Re: i2c-core: One function call less in acpi_i2c_space_handler() after error detection

2015-12-26 Thread SF Markus Elfring
>> I would appreciate if an unnecessary function call can be avoided here >> so that the affected exception handling can become also a bit more efficient. > > Simpler code is easier to maintain. There are different opinions available around the desired simplicity. > See your patch, you didn't

[PATCH 1/3] IDE-ACPI: One function call less in ide_get_dev_handle() after error detection

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 10:01:36 +0100 The kfree() function was called in two cases by the ide_get_dev_handle() function during error handling even if the passed variable "dinfo" contained a null pointer. * Let us return directly if a call of

[PATCH 2/3] IDE-ACPI: Delete unnecessary null pointer checks in ide_get_dev_handle()

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 10:33:48 +0100 The variable "dinfo" will contain an appropropriate pointer after a call of the acpi_get_object_info() function succeeded. Thus remove two safety checks. Signed-off-by: Markus Elfring

Re: i2c-core: One function call less in acpi_i2c_space_handler() after error detection

2015-12-26 Thread SF Markus Elfring
>> The kfree() function was called in one case by the >> acpi_i2c_space_handler() function during error handling >> even if the passed variable "client" contained a null pointer. > > This is OK. kfree() is known to be NULL-tolerant and we rely on it in > various places to keep the code simpler.

[PATCH 3/3] IDE-ACPI: Move an assignment for one variable in ide_get_dev_handle()

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 10:50:33 +0100 Move the assignment for the variable "addr" to the statement where its value is used after previous function calls succeeded. Signed-off-by: Markus Elfring ---

[PATCH] iio: qcom-spmi-vadc: One check less in vadc_measure_ref_points() after error detection

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 13:53:15 +0100 This issue was detected by using the Coccinelle software. Move the jump label directly before the desired log statement so that the variable "ret" does not need to be checked once more after it was

[PATCH 0/3] IDE-ACPI: Fine-tuning for a function

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 11:04:56 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (3): One function call less in ide_get_dev_handle() after error detection Delete unnecessary null

[PATCH v2 1/6] InfiniBand-ocrdma: One jump label less in ocrdma_alloc_ucontext_pd()

2015-12-26 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 26 Dec 2015 22:18:38 +0100 This issue was detected by using the Coccinelle software. * Let us return directly if a call of the _ocrdma_alloc_pd() function failed. * Reduce the scope for the local variable "status" to one case

Re: Documentation-getdelays: Less function calls in usage()

2015-12-24 Thread SF Markus Elfring
>> A single call of the fprintf() function is sufficient for the desired >> display of the usage information. > > This seems like churn that doesn't actually fix anything. Will it matter occasionally to reduce the number of function calls by combining several text fragments into the passing of

[PATCH] SCSI-lpfc: Use a signed return type for two functions

2015-12-19 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 19 Dec 2015 19:32:27 +0100 The return type "size_t" was used by the functions "lpfc_wwn_set" and "lpfc_oas_lun_state_set" despite of the aspect that they will eventually return a negative error code. Improve this implementation

[PATCH] USB-FHCI: Use a signed return type for fhci_create_ep()

2015-12-19 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 19 Dec 2015 21:10:20 +0100 The return type "u32" was used by the fhci_create_ep() function even though it will eventually return a negative error code. Improve this implementation detail by using the type "s32" instead. This issue

Re: pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

2015-12-19 Thread SF Markus Elfring
>>> This introduces a compile warning. >> >> How do you think about to show the exact message you get? > > I can't actually compile it myself. It seems that I can understand your feedback also a bit better since I received the information from a background process like "kbuild test robot". Will

[PATCH] pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

2015-12-19 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 19 Dec 2015 17:55:39 +0100 The return type "unsigned int" was used by the adi_gpio_irq_startup() function despite of the aspect that it will eventually return a negative error code. Improve this implementation detail by deletion of

Re: [PATCH] USB-FHCI: Use a signed return type for fhci_create_ep()

2015-12-19 Thread SF Markus Elfring
> Just make it an int. Thanks for your suggestion. Will any more software developers prefer this data type at some source code places? > The caller also casts it to u32... Do you want to get rid of similar casts in affected functions? Regards, Markus -- To unsubscribe from this list: send

[PATCH] ti-st: Use a signed return type for st_ll_sleep_state()

2015-12-19 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 19 Dec 2015 17:15:34 +0100 The return type "unsigned long" was used by the st_ll_sleep_state() function despite of the aspect that it will eventually return a negative error code. Improve this implementation detail by deletion of the

[PATCH] staging-slicoss: Use a signed return type for slic_card_locate()

2015-12-19 Thread SF Markus Elfring
From: Markus Elfring Date: Sat, 19 Dec 2015 20:30:39 +0100 The return type "u32" was used by the slic_card_locate() function despite of the aspect that it will eventually return a negative error code. Improve this implementation detail by using the type "s32"

Re: pinctrl-adi2: Use a signed return type for adi_gpio_irq_startup()

2015-12-19 Thread SF Markus Elfring
> This introduces a compile warning. How do you think about to show the exact message you get? > These functions are supposed to return 1 if there is an IRQ pending. > Change the -EINVAL to 0. Is there any more source code clean-up needed around the comment "FIXME" in the affected function?

[PATCH] blackfin-cpufreq: Change return type of cpu_set_cclk() to that of clk_set_rate()

2015-12-18 Thread SF Markus Elfring
From: Markus Elfring Date: Fri, 18 Dec 2015 19:43:27 +0100 The return type "unsigned long" was used by the cpu_set_cclk() function while the type "int" is provided by the clk_set_rate() function. Let us make this usage consistent. This issue was detected by using

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

2015-12-20 Thread SF Markus Elfring
Reuse the type from this poll call instead. >>> >>> Why use uint when the function return type it unsigned int? >> >> Do you prefer to express the type modifier once more there? > > I don't know what the sentence means, Can it be a matter of taste if the key word "unsigned" should be

[PATCH] XFS: Use a signed return type for suffix_kstrtoint()

2015-12-19 Thread SF Markus Elfring
From: Markus Elfring Date: Sun, 20 Dec 2015 07:56:36 +0100 The return type "unsigned long" was used by the suffix_kstrtoint() function even though it will eventually return a negative error code. Improve this implementation detail by using the type "int" instead.

[PATCH] posix-clock: Use an unsigned data type for a variable

2015-12-20 Thread SF Markus Elfring
From: Markus Elfring Date: Sun, 20 Dec 2015 09:09:34 +0100 The data type "int" was used by the variable "result" in the function "posix_clock_poll" even though the type "uint" will usually be needed for the return value from a call of the function which was

Re: [PATCH] posix-clock: Use an unsigned data type for a variable

2015-12-20 Thread SF Markus Elfring
>> Reuse the type from this poll call instead. > > Why use uint when the function return type it unsigned int? Do you prefer to express the type modifier once more there? > On the other hand, why is the function return type unsigned int > when there is a return of a negative constant? This

[PATCH] ASoC: ssm2518: Use a signed return type for ssm2518_lookup_mcs()

2015-12-20 Thread SF Markus Elfring
From: Markus Elfring Date: Sun, 20 Dec 2015 10:34:25 +0100 The return type "unsigned int" was used by the ssm2518_lookup_mcs() function even though it will eventually return a negative error code. Improve this implementation detail by deletion of the type modifier

Re: [Cocci] [PATCH v2] coccinelle: api: check for propagation of error from platform_get_irq

2015-12-27 Thread SF Markus Elfring
>> https://cwe.mitre.org/data/definitions/252.html > > The value is not unchecked. Would you like to express any stronger relationship between the function call example and the occurrence of an if statement by the discussed SmPL script? > I made a specific rule because the specific problem is

[PATCH] [media] bttv: Returning only value constants in two functions

2015-12-27 Thread SF Markus Elfring
From: Markus Elfring Date: Sun, 27 Dec 2015 22:02:21 +0100 Return constant integer values without storing them in the local variable "err" or "rc". Signed-off-by: Markus Elfring --- drivers/media/pci/bt8xx/bttv-driver.c | 25

[PATCH] [media] si2165: Refactoring for si2165_writereg_mask8()

2015-12-27 Thread SF Markus Elfring
From: Markus Elfring Date: Sun, 27 Dec 2015 18:23:57 +0100 This issue was detected by using the Coccinelle software. 1. Let us return directly if a call of the si2165_readreg8() function failed. 2. Reduce the scope for the local variables "ret" and "tmp" to

[PATCH] [media] au0828: Refactoring for start_urb_transfer()

2015-12-28 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 28 Dec 2015 22:52:48 +0100 This issue was detected by using the Coccinelle software. 1. Let us return directly if a buffer allocation failed. 2. Delete the jump label "err" then. 3. Drop the explicit initialisation for the

Re: Documentation-getdelays: Fix a check for container file usage in main()

2015-12-24 Thread SF Markus Elfring
>> The close() function could be called by the main() function even if >> the passed variable "cfd" was assigned a negative value. … > This seems more easily fixed by simply making the condition > 0. How do you think about the reuse of the error predicate "cfd != -1" for the return value from a

Re: [Cocci] [PATCH v2] coccinelle: api: check for propagation of error from platform_get_irq

2015-12-26 Thread SF Markus Elfring
> The error return value of platform_get_irq seems to often get dropped. How do you think about any more fine-tuning here? Commit message: * … of the platform_get_irq() function seems to get dropped too often. * Why do you concentrate on a single function name? Do you plan to extend this

Re: [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection

2015-12-28 Thread SF Markus Elfring
>> Move the jump label directly before the desired log statement >> so that the variable "ret" will not be checked once more >> after it was determined that a function call failed. > > Why not avoid both unnecessary ifs I would find such a fine-tuning also nice in principle at more source code

[PATCH] [media] tuners: One check less in m88rs6000t_get_rf_strength() after error detection

2015-12-28 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 28 Dec 2015 10:10:34 +0100 This issue was detected by using the Coccinelle software. Move the jump label directly before the desired log statement so that the variable "ret" will not be checked once more after it was determined that

[PATCH 1/2] [media] m88rs6000t: Better exception handling in five functions

2015-12-28 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 28 Dec 2015 15:10:30 +0100 This issue was detected by using the Coccinelle software. Move the jump label directly before the desired log statement so that the variable "ret" will not be checked once more after a function call. Use

[PATCH 2/2] [media] tuners: Refactoring for m88rs6000t_sleep()

2015-12-28 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 28 Dec 2015 15:20:45 +0100 This issue was detected by using the Coccinelle software. 1. Let us return directly if a call of the regmap_write() function failed. 2. Delete the jump label "err" then. 3. Return zero as a constant at

[PATCH 0/2] [media] m88rs6000t: Fine-tuning for some function implementations

2015-12-28 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 28 Dec 2015 15:32:20 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (2): Better exception handling in five functions Refactoring for m88rs6000t_sleep()

Re: [media] m88rs6000t: Better exception handling in five functions

2015-12-28 Thread SF Markus Elfring
>> Move the jump label directly before the desired log statement >> so that the variable "ret" will not be checked once more >> after a function call. > > This commit message fits with the previous change. Do you prefer an other wording? > It could be nice to put a blank line before the error

[PATCH 1/2] [media] r820t: Delete an unnecessary variable initialisation in generic_set_freq()

2015-12-28 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 28 Dec 2015 16:36:44 +0100 The variable "rc" will be set to an appropriate value from a call of the r820t_set_tv_standard() function. Thus let us omit the explicit initialisation at the beginning. Signed-off-by: Markus Elfring

[PATCH 2/2] [media] r820t: Better exception handling in generic_set_freq()

2015-12-28 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 28 Dec 2015 17:13:02 +0100 This issue was detected by using the Coccinelle software. Move the jump label directly before the desired log statement so that the variable "rc" will not be checked once more after a function call. Use

[PATCH 0/2] [media] r820t: Fine-tuning for generic_set_freq()

2015-12-28 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 28 Dec 2015 17:18:34 +0100 A few update suggestions were taken into account from static source code analysis. Markus Elfring (2): Delete an unnecessary variable initialisation Better exception handling

[PATCH 0/2] InfiniBand-iSER: Refactoring for two function implementations

2015-12-27 Thread SF Markus Elfring
From: Markus Elfring Date: Sun, 27 Dec 2015 13:12:10 +0100 Subject: [PATCH 0/2] InfiniBand-iSER: Refactoring for two function implementations I suggest to return directly instead of using the jump label "err" in two functions (which are working without clean-up

[PATCH 1/2] InfiniBand-iSER: One jump label less in iser_reg_sig_mr()

2015-12-27 Thread SF Markus Elfring
From: Markus Elfring Date: Sun, 27 Dec 2015 11:41:42 +0100 This issue was detected by using the Coccinelle software. 1. Let us return directly if a call of the iser_set_sig_attrs() function failed. 2. Delete the jump label "err" then. 3. Return zero as a

[PATCH 2/2] InfiniBand-iSER-target: One jump label less in isert_reg_sig_mr()

2015-12-27 Thread SF Markus Elfring
From: Markus Elfring Date: Sun, 27 Dec 2015 12:54:52 +0100 This issue was detected by using the Coccinelle software. 1. Let us return directly if a call of the function "isert_set_sig_attrs" or "ib_post_send" failed. 2. Delete the jump label "err" then. 3.

[PATCH] [media] airspy: Better exception handling in two functions

2015-12-28 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 28 Dec 2015 22:10:28 +0100 This issue was detected by using the Coccinelle software. Move the jump label directly before the desired log statement so that the variable "ret" will not be checked once more after a function call. Use

[PATCH] [media] xc5000: Faster result reporting in xc_load_fw_and_init_tuner()

2015-12-28 Thread SF Markus Elfring
From: Markus Elfring Date: Mon, 28 Dec 2015 20:10:30 +0100 This issue was detected by using the Coccinelle software. Split the previous if statement at the end so that each final log statement will eventually be performed by a direct jump to these labels. *

Re: blackfin-cpufreq: How to mark cpu_set_cclk() as static?

2015-12-21 Thread SF Markus Elfring
> I think the first patch is already Acked now and can be applied > as it is. Just send the next patch based on the previous one. Does it matter to provide the proposed changes as a tiny patch series with two update steps? Regards, Markus -- To unsubscribe from this list: send the line

Re: blackfin-cpufreq: How to mark cpu_set_cclk() as static?

2015-12-21 Thread SF Markus Elfring
> This should be static, yeah. Would you like to integrate a corresponding small source code change yourself? (Do you need a separate patch from me?) Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org

Re: posix-clock: Use an unsigned data type for a variable

2015-12-21 Thread SF Markus Elfring
> In this case, there is indeed a problem. However, just changing the > type won't fix it. Would you like to explain your view on the handling of potential data type mismatches a bit more? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of

Re: blackfin-cpufreq: How to mark cpu_set_cclk() as static?

2015-12-21 Thread SF Markus Elfring
> Since you reported the issue, it will be good if you can send a patch > for this as well. I'm sorry if I become a bit picky for this implementation detail. In which order would you prefer that the properties of the function "cpu_set_cclk" will be improved? * Should the linkage specifier be

Re: [PATCH v2 3/4] staging: lustre: Less checks in mgc_process_recover_log() after error detection

2015-12-21 Thread SF Markus Elfring
>> 6. Apply a recommendation from the script "checkpatch.pl". > > That's 6 different things, shouldn't this be 6 different patches? > > please redo. Dan Carpenter requested to squash the previous update steps 5 and 6 into a single patch for better source code review. Now I see further software

<    4   5   6   7   8   9   10   11   12   13   >