clk: change clk_ops' ->determine_rate() prototype

Hello Boris Brezillon,

This is a semi-automatic email about new static checker warnings.

The patch 4a7c639697ef: "clk: change clk_ops' ->determine_rate() prototype" from Jul 7, 2015, leads to the following Smatch complaint:

drivers/clk/clk.c:458 clk_mux_determine_rate_flags()
error: we previously assumed 'parent' could be null (see line 452)

drivers/clk/clk.c

    451			if (core->flags & CLK_SET_RATE_PARENT) {
    452				ret = __clk_determine_rate(parent ? parent->hw : NULL,
                                                            ^^^^^^

Check for NULL.

    453							   &parent_req);
    454				if (ret)
    455					return ret;
    456	
    457				best = parent_req.rate;
    458				req->best_parent_hw = parent->hw;
                                                       ^^^^^^^^^^

Patch adds unchecked dereference.

    459				req->best_parent_rate = best;
    460			} else if (parent) {

Quoted 16 lines · Dan Carpenter

Hello Boris Brezillon,

This is a semi-automatic email about new static checker warnings.

The patch 4a7c639697ef: "clk: change clk_ops' ->determine_rate() prototype" from Jul 7, 2015, leads to the following Smatch complaint:

drivers/clk/clk.c:458 clk_mux_determine_rate_flags()
error: we previously assumed 'parent' could be null (see line 452)

drivers/clk/clk.c

    451			if (core->flags & CLK_SET_RATE_PARENT) {
    452				ret = __clk_determine_rate(parent ? parent->hw : NULL,
                                                            ^^^^^^

Check for NULL.

Quoted 8 lines

    453							   &parent_req);
    454				if (ret)
    455					return ret;
    456	
    457				best = parent_req.rate;
    458				req->best_parent_hw = parent->hw;
                                                       ^^^^^^^^^^

Patch adds unchecked dereference.

Quoted 5 lines

    459				req->best_parent_rate = best;
    460			} else if (parent) {

Thanks Dan.

Boris,

It looks like the transformation here isn't equivalent. If parent is NULL here and we don't crash, parent_req->rate == req->rate and before this patch req->rate would be 0 instead. How about this patch squashed in? It includes some of the removals of best_parent_* assignments that you had in your follow-up patch, because those make it equivalent to the previous code.

------8<----

drivers/clk/clk.c +3 -7

index c182d8a7924e..244a6535c69c 100644
@@ -455,16 +455,10 @@ clk_mux_determine_rate_flags(struct clk_hw *hw, struct clk_rate_request *req,
  				return ret;
 
  			best = parent_req.rate;
-			req->best_parent_hw = parent->hw;
-			req->best_parent_rate = best;
  		} else if (parent) {
  			best = clk_core_get_rate_nolock(parent);
-			req->best_parent_hw = parent->hw;
-			req->best_parent_rate = best;
  		} else {
  			best = clk_core_get_rate_nolock(core);
-			req->best_parent_hw = NULL;
-			req->best_parent_rate = 0;
  		}
 
  		goto out;

@@ -810,8 +804,10 @@ static int clk_core_round_rate_nolock(struct clk_core *core,
   */
  int __clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
  {
-	if (!hw)
+	if (!hw) {
+		req->rate = 0;
  		return 0;
+	}
 
  	return clk_core_round_rate_nolock(hw->core, req);
  }