diff mbox series

clk: ti: dra7-atl: don't allocate `parent_names' variable

Message ID 20221017165028.4194254-1-dario.binacchi@amarulasolutions.com
State New
Headers
Series clk: ti: dra7-atl: don't allocate `parent_names' variable |

Commit Message

Dario Binacchi Oct. 17, 2022, 4:50 p.m. UTC 
The `parent_names' variable was freed also in case of kzalloc() error.
Instead of modifying the code to perform a proper memory release, I
decided to fix the bug by not allocating memory.
Since only one parent name is referenced, it is not necessary to
allocate this variable at runtime and therefore you can avoid calling
the kzalloc() function. This simplifies the code (even calls to kfree
can be removed) and improves the performance of the routine.

Note: Although no operation is performed by kfree() on a NULL pointer,
it was however suboptimal and semantically wrong doing it.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>

---

 drivers/clk/ti/clk-dra7-atl.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

Comments

kernel test robot Oct. 18, 2022, 2:07 a.m. UTC | #1 
Hi Dario,

I love your patch! Yet something to improve:

[auto build test ERROR on clk/clk-next]
[also build test ERROR on linus/master v6.1-rc1 next-20221017]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Dario-Binacchi/clk-ti-dra7-atl-don-t-allocate-parent_names-variable/20221018-005059
base:   https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link:    https://lore.kernel.org/r/20221017165028.4194254-1-dario.binacchi%40amarulasolutions.com
patch subject: [PATCH] clk: ti: dra7-atl: don't allocate `parent_names' variable
config: arm-defconfig
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/9eca9eab009687ea2b8cc0d262de7786b6d449b9
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Dario-Binacchi/clk-ti-dra7-atl-don-t-allocate-parent_names-variable/20221018-005059
        git checkout 9eca9eab009687ea2b8cc0d262de7786b6d449b9
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers/clk/ti/clk-dra7-atl.c: In function 'of_dra7_atl_clock_setup':
>> drivers/clk/ti/clk-dra7-atl.c:190:27: error: assignment to 'const char * const*' from incompatible pointer type 'const char *' [-Werror=incompatible-pointer-types]
     190 |         init.parent_names = of_clk_get_parent_name(node, 0);
         |                           ^
   cc1: some warnings being treated as errors


vim +190 drivers/clk/ti/clk-dra7-atl.c

   162	
   163	static void __init of_dra7_atl_clock_setup(struct device_node *node)
   164	{
   165		struct dra7_atl_desc *clk_hw = NULL;
   166		struct clk_init_data init = { NULL };
   167		const char *name;
   168		struct clk *clk;
   169	
   170		clk_hw = kzalloc(sizeof(*clk_hw), GFP_KERNEL);
   171		if (!clk_hw) {
   172			pr_err("%s: could not allocate dra7_atl_desc\n", __func__);
   173			return;
   174		}
   175	
   176		clk_hw->hw.init = &init;
   177		clk_hw->divider = 1;
   178		name = ti_dt_clk_name(node);
   179		init.name = name;
   180		init.ops = &atl_clk_ops;
   181		init.flags = CLK_IGNORE_UNUSED;
   182		init.num_parents = of_clk_get_parent_count(node);
   183	
   184		if (init.num_parents != 1) {
   185			pr_err("%s: atl clock %pOFn must have 1 parent\n", __func__,
   186			       node);
   187			goto cleanup;
   188		}
   189	
 > 190		init.parent_names = of_clk_get_parent_name(node, 0);
   191	
   192		clk = ti_clk_register(NULL, &clk_hw->hw, name);
   193		if (!IS_ERR(clk)) {
   194			of_clk_add_provider(node, of_clk_src_simple_get, clk);
   195			return;
   196		}
   197	
   198	cleanup:
   199		kfree(clk_hw);
   200	}
   201	CLK_OF_DECLARE(dra7_atl_clock, "ti,dra7-atl-clock", of_dra7_atl_clock_setup);
   202
diff mbox series

Patch

diff --git a/drivers/clk/ti/clk-dra7-atl.c b/drivers/clk/ti/clk-dra7-atl.c
index ff4d6a951681..ec2473069143 100644
--- a/drivers/clk/ti/clk-dra7-atl.c
+++ b/drivers/clk/ti/clk-dra7-atl.c
@@ -164,7 +164,6 @@  static void __init of_dra7_atl_clock_setup(struct device_node *node)
 {
 	struct dra7_atl_desc *clk_hw = NULL;
 	struct clk_init_data init = { NULL };
-	const char **parent_names = NULL;
 	const char *name;
 	struct clk *clk;
 
@@ -188,24 +187,15 @@  static void __init of_dra7_atl_clock_setup(struct device_node *node)
 		goto cleanup;
 	}
 
-	parent_names = kzalloc(sizeof(char *), GFP_KERNEL);
-
-	if (!parent_names)
-		goto cleanup;
-
-	parent_names[0] = of_clk_get_parent_name(node, 0);
-
-	init.parent_names = parent_names;
+	init.parent_names = of_clk_get_parent_name(node, 0);
 
 	clk = ti_clk_register(NULL, &clk_hw->hw, name);
-
 	if (!IS_ERR(clk)) {
 		of_clk_add_provider(node, of_clk_src_simple_get, clk);
-		kfree(parent_names);
 		return;
 	}
+
 cleanup:
-	kfree(parent_names);
 	kfree(clk_hw);
 }
 CLK_OF_DECLARE(dra7_atl_clock, "ti,dra7-atl-clock", of_dra7_atl_clock_setup);