On Mon, 2015-03-30 at 13:50 +0300, David Weinehall wrote:
> On Fri, Mar 27, 2015 at 05:36:40PM +0300, Dan Carpenter wrote:
> > Hello Ander Conselvan de Oliveira,
> >
> > This is a semi-automatic email about new static checker warnings.
> >
> > The patch 944b0c765757: "drm/i915: Copy the staged connector config
> > to the legacy atomic state" from Mar 20, 2015, leads to the following
> > Smatch complaint:
> >
> > drivers/gpu/drm/i915/intel_display.c:9118 intel_release_load_detect_pipe()
> > error: we previously assumed 'state' could be null (see line 9082)
> >
> > drivers/gpu/drm/i915/intel_display.c
> > 9081 state = drm_atomic_state_alloc(dev);
> > 9082 if (!state)
> > 9083 goto fail;
> > ^^^^^^^^^
> > Patch changes a return into a goto.
> >
> > 9084
> > 9085 state->acquire_ctx = ctx;
> > 9086
> > 9087 connector_state = drm_atomic_get_connector_state(state, connector);
> > 9088 if (IS_ERR(connector_state))
> > 9089 goto fail;
> > 9090
> > 9091 to_intel_connector(connector)->new_encoder = NULL;
> > 9092 intel_encoder->new_crtc = NULL;
> > 9093 intel_crtc->new_enabled = false;
> > 9094 intel_crtc->new_config = NULL;
> > 9095
> > 9096 connector_state->best_encoder = NULL;
> > 9097 connector_state->crtc = NULL;
> > 9098
> > 9099 intel_set_mode(crtc, NULL, 0, 0, NULL, state);
> > 9100
> > 9101 drm_atomic_state_free(state);
> > 9102
> > 9103 if (old->release_fb) {
> > 9104 drm_framebuffer_unregister_private(old->release_fb);
> > 9105 drm_framebuffer_unreference(old->release_fb);
> > 9106 }
> > 9107
> > 9108 return;
> > 9109 }
> > 9110
> > 9111 /* Switch crtc and encoder back off if necessary */
> > 9112 if (old->dpms_mode != DRM_MODE_DPMS_ON)
> > 9113 connector->funcs->dpms(connector, old->dpms_mode);
> > 9114
> > 9115 return;
> > 9116 fail:
> > 9117 DRM_DEBUG_KMS("Couldn't release load detect pipe.\n");
> > 9118 drm_atomic_state_free(state);
> > ^^^^^
> > Dereferenced inside function call.
> >
> > 9119 }
> > 9120
>
> Wouldn't it make more sense to make drm_atomic_state_free() follow
> the same semantics as *free() functions typically do
> (no operation performed)?
It would. I already signed up to send that patch, even. Just didn't get
around to yet.
Ander
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
