[bug report] drm/amd/dc: Add dc display driver (v2)

Dan Carpenter dan.carpenter at oracle.com
Wed Jan 9 19:29:43 UTC 2019


Hello Harry Wentland,

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

The patch 4562236b3bc0: "drm/amd/dc: Add dc display driver (v2)" from 
Sep 12, 2017, leads to the following Smatch complaint:

    drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c:754 construct()
    error: we previously assumed 'dc->current_state' could be null (see line 677)

drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.c
   676	
   677		if (!dc->current_state) {
                    ^^^^^^^^^^^^^^^^^^

   678			dm_error("%s: failed to create validate ctx\n", __func__);
   679			goto fail;
                        ^^^^^^^^^

   680		}
   681	
   682		/* Create logger */
   683	
   684		dc_ctx->dce_environment = init_params->dce_environment;
   685	
   686		dc_version = resource_parse_asic_id(init_params->asic_id);
   687		dc_ctx->dce_version = dc_version;
   688	
   689		/* Resource should construct all asic specific resources.
   690		 * This should be the only place where we need to parse the asic id
   691		 */
   692		if (init_params->vbios_override)
   693			dc_ctx->dc_bios = init_params->vbios_override;
   694		else {
   695			/* Create BIOS parser */
   696			struct bp_init_data bp_init_data;
   697	
   698			bp_init_data.ctx = dc_ctx;
   699			bp_init_data.bios = init_params->asic_id.atombios_base_address;
   700	
   701			dc_ctx->dc_bios = dal_bios_parser_create(
   702					&bp_init_data, dc_version);
   703	
   704			if (!dc_ctx->dc_bios) {
   705				ASSERT_CRITICAL(false);
   706				goto fail;
   707			}
   708	
   709			dc_ctx->created_bios = true;
   710			}
   711	
   712		/* Create I2C AUX */
   713		dc_ctx->i2caux = dal_i2caux_create(dc_ctx);
   714	
   715		if (!dc_ctx->i2caux) {
   716			ASSERT_CRITICAL(false);
   717			goto fail;
   718		}
   719	
   720		dc_ctx->perf_trace = dc_perf_trace_create();
   721		if (!dc_ctx->perf_trace) {
   722			ASSERT_CRITICAL(false);
   723			goto fail;
   724		}
   725	
   726		/* Create GPIO service */
   727		dc_ctx->gpio_service = dal_gpio_service_create(
   728				dc_version,
   729				dc_ctx->dce_environment,
   730				dc_ctx);
   731	
   732		if (!dc_ctx->gpio_service) {
   733			ASSERT_CRITICAL(false);
   734			goto fail;
   735		}
   736	
   737		dc->res_pool = dc_create_resource_pool(
   738				dc,
   739				init_params->num_virtual_links,
   740				dc_version,
   741				init_params->asic_id);
   742		if (!dc->res_pool)
   743			goto fail;
   744	
   745		dc_resource_state_construct(dc, dc->current_state);
   746	
   747		if (!create_links(dc, init_params->num_virtual_links))
   748			goto fail;
   749	
   750		return true;
   751	
   752	fail:
   753	
   754		destruct(dc);
                         ^^
"dc->current_state" gets dereferenced inside the function.  This style
of one function cleans up everything error handling is always buggy...

:/

https://plus.google.com/u/0/106378716002406849458/posts/1Ud9JbaYnPr

   755		return false;
   756	}

regards,
dan carpenter


More information about the amd-gfx mailing list