On Mon, Apr 16, 2012 at 1:30 PM, Sascha Hauer s.hauer@pengutronix.de wrote:
On Wed, Apr 11, 2012 at 06:02:50PM -0700, Mike Turquette wrote:
struct clk *clk_register(struct device *dev, const char *name, const struct clk_ops *ops, struct clk_hw *hw, const char **parent_names, u8 num_parents, unsigned long flags) {
- int i, ret = -ENOMEM;
I suggest to move the initialization of ret from here...
struct clk *clk;
clk = kzalloc(sizeof(*clk), GFP_KERNEL);
- if (!clk)
- return NULL;
- if (!clk) {
- pr_err("%s: could not allocate clk\n", __func__);
- goto fail_out;
- }
clk->name = name; clk->ops = ops; clk->hw = hw; clk->flags = flags;
- clk->parent_names = parent_names;
clk->num_parents = num_parents; hw->clk = clk;
- __clk_init(dev, clk);
- /* allocate local copy in case parent_names is __initdata */
- clk->parent_names = kzalloc((sizeof(char*) * num_parents),
- GFP_KERNEL);
- if (!clk->parent_names) {
- pr_err("%s: could not allocate clk->parent_names\n", __func__);
- goto fail_parent_names;
- }
- /* copy each string name in case parent_names is __initdata */
... to here.
The rationale is that when this code is changed later someone might use ret above and doesn't remember that the code below expects ret to be initialized with -ENOMEM. Also it's easier to see that the code is correct.
That is sensible.
Thanks, Mike
Sascha