public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH RESEND 1/3] drm/i915: use error path
@ 2015-10-08 13:57 Sudip Mukherjee
  2015-10-08 13:58 ` [PATCH RESEND 2/3] drm/i915: check for return value Sudip Mukherjee
  2015-10-08 14:30 ` [PATCH RESEND 1/3] drm/i915: use error path Daniel Vetter
  0 siblings, 2 replies; 6+ messages in thread
From: Sudip Mukherjee @ 2015-10-08 13:57 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter
  Cc: intel-gfx, linux-kernel, dri-devel, Daniel Vetter,
	Sudip Mukherjee

Use goto to handle the error path to avoid duplicating the same code. In
the error path intel_dig_port is the last one to be released as it was
the first one to be allocated and ideally the error path should be the
reverse of the execution path.

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

Sent on 27/07/2015

 drivers/gpu/drm/i915/intel_dp.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8d34ca7..18bcfbe 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6168,10 +6168,8 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 		return;
 
 	intel_connector = intel_connector_alloc();
-	if (!intel_connector) {
-		kfree(intel_dig_port);
-		return;
-	}
+	if (!intel_connector)
+		goto err_connector_alloc;
 
 	intel_encoder = &intel_dig_port->base;
 	encoder = &intel_encoder->base;
@@ -6219,11 +6217,18 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
 	dev_priv->hotplug.irq_port[port] = intel_dig_port;
 
-	if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
-		drm_encoder_cleanup(encoder);
-		kfree(intel_dig_port);
-		kfree(intel_connector);
-	}
+	if (!intel_dp_init_connector(intel_dig_port, intel_connector))
+		goto err_init_connector;
+
+	return;
+
+err_init_connector:
+	drm_encoder_cleanup(encoder);
+	kfree(intel_connector);
+err_connector_alloc:
+	kfree(intel_dig_port);
+
+	return;
 }
 
 void intel_dp_mst_suspend(struct drm_device *dev)
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH RESEND 2/3] drm/i915: check for return value
  2015-10-08 13:57 [PATCH RESEND 1/3] drm/i915: use error path Sudip Mukherjee
@ 2015-10-08 13:58 ` Sudip Mukherjee
  2015-10-08 14:29   ` Daniel Vetter
  2015-10-08 14:30 ` [PATCH RESEND 1/3] drm/i915: use error path Daniel Vetter
  1 sibling, 1 reply; 6+ messages in thread
From: Sudip Mukherjee @ 2015-10-08 13:58 UTC (permalink / raw)
  To: David Airlie, Daniel Vetter
  Cc: intel-gfx, linux-kernel, dri-devel, Daniel Vetter,
	Sudip Mukherjee

We were not checking the return value of drm_encoder_init() which can
fail. And if it fails then we will be working with an uninitialized
encoder.

Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

Sent on 27/07/2015

 drivers/gpu/drm/i915/intel_dp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 18bcfbe..2a8b47e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -6174,8 +6174,9 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 	intel_encoder = &intel_dig_port->base;
 	encoder = &intel_encoder->base;
 
-	drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
-			 DRM_MODE_ENCODER_TMDS);
+	if (drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
+			     DRM_MODE_ENCODER_TMDS))
+		goto err_encoder_init;
 
 	intel_encoder->compute_config = intel_dp_compute_config;
 	intel_encoder->disable = intel_disable_dp;
@@ -6224,6 +6225,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
 
 err_init_connector:
 	drm_encoder_cleanup(encoder);
+err_encoder_init:
 	kfree(intel_connector);
 err_connector_alloc:
 	kfree(intel_dig_port);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH RESEND 2/3] drm/i915: check for return value
  2015-10-08 13:58 ` [PATCH RESEND 2/3] drm/i915: check for return value Sudip Mukherjee
@ 2015-10-08 14:29   ` Daniel Vetter
  2015-12-09 11:39     ` Sudip Mukherjee
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2015-10-08 14:29 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: Daniel Vetter, intel-gfx, linux-kernel, dri-devel, Daniel Vetter

On Thu, Oct 08, 2015 at 07:28:00PM +0530, Sudip Mukherjee wrote:
> We were not checking the return value of drm_encoder_init() which can
> fail. And if it fails then we will be working with an uninitialized
> encoder.
> 
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>

Queued for -next, thanks for the patch.
-Daniel

> ---
> 
> Sent on 27/07/2015
> 
>  drivers/gpu/drm/i915/intel_dp.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 18bcfbe..2a8b47e 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6174,8 +6174,9 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  	intel_encoder = &intel_dig_port->base;
>  	encoder = &intel_encoder->base;
>  
> -	drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
> -			 DRM_MODE_ENCODER_TMDS);
> +	if (drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
> +			     DRM_MODE_ENCODER_TMDS))
> +		goto err_encoder_init;
>  
>  	intel_encoder->compute_config = intel_dp_compute_config;
>  	intel_encoder->disable = intel_disable_dp;
> @@ -6224,6 +6225,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  
>  err_init_connector:
>  	drm_encoder_cleanup(encoder);
> +err_encoder_init:
>  	kfree(intel_connector);
>  err_connector_alloc:
>  	kfree(intel_dig_port);
> -- 
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH RESEND 1/3] drm/i915: use error path
  2015-10-08 13:57 [PATCH RESEND 1/3] drm/i915: use error path Sudip Mukherjee
  2015-10-08 13:58 ` [PATCH RESEND 2/3] drm/i915: check for return value Sudip Mukherjee
@ 2015-10-08 14:30 ` Daniel Vetter
  1 sibling, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2015-10-08 14:30 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: David Airlie, Daniel Vetter, intel-gfx, linux-kernel, dri-devel,
	Daniel Vetter

On Thu, Oct 08, 2015 at 07:27:59PM +0530, Sudip Mukherjee wrote:
> Use goto to handle the error path to avoid duplicating the same code. In
> the error path intel_dig_port is the last one to be released as it was
> the first one to be allocated and ideally the error path should be the
> reverse of the execution path.
> 
> Cc: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>

Queued for -next, thanks for the patch.
-Daniel

> ---
> 
> Sent on 27/07/2015
> 
>  drivers/gpu/drm/i915/intel_dp.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 8d34ca7..18bcfbe 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6168,10 +6168,8 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  		return;
>  
>  	intel_connector = intel_connector_alloc();
> -	if (!intel_connector) {
> -		kfree(intel_dig_port);
> -		return;
> -	}
> +	if (!intel_connector)
> +		goto err_connector_alloc;
>  
>  	intel_encoder = &intel_dig_port->base;
>  	encoder = &intel_encoder->base;
> @@ -6219,11 +6217,18 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
>  	intel_dig_port->hpd_pulse = intel_dp_hpd_pulse;
>  	dev_priv->hotplug.irq_port[port] = intel_dig_port;
>  
> -	if (!intel_dp_init_connector(intel_dig_port, intel_connector)) {
> -		drm_encoder_cleanup(encoder);
> -		kfree(intel_dig_port);
> -		kfree(intel_connector);
> -	}
> +	if (!intel_dp_init_connector(intel_dig_port, intel_connector))
> +		goto err_init_connector;
> +
> +	return;
> +
> +err_init_connector:
> +	drm_encoder_cleanup(encoder);
> +	kfree(intel_connector);
> +err_connector_alloc:
> +	kfree(intel_dig_port);
> +
> +	return;
>  }
>  
>  void intel_dp_mst_suspend(struct drm_device *dev)
> -- 
> 1.9.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH RESEND 2/3] drm/i915: check for return value
  2015-10-08 14:29   ` Daniel Vetter
@ 2015-12-09 11:39     ` Sudip Mukherjee
  2015-12-09 13:33       ` Daniel Vetter
  0 siblings, 1 reply; 6+ messages in thread
From: Sudip Mukherjee @ 2015-12-09 11:39 UTC (permalink / raw)
  To: David Airlie, linux-kernel, dri-devel, intel-gfx, Daniel Vetter,
	Jani Nikula

On Thu, Oct 08, 2015 at 04:29:31PM +0200, Daniel Vetter wrote:
> On Thu, Oct 08, 2015 at 07:28:00PM +0530, Sudip Mukherjee wrote:
> > We were not checking the return value of drm_encoder_init() which can
> > fail. And if it fails then we will be working with an uninitialized
> > encoder.
> > 
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> 
> Queued for -next, thanks for the patch.
> -Daniel

Hi Daniel,
It is still not there in linux-next. Still applies cleanly on
next-20151209.

regards
sudip

> 
> > ---
> > 
> > Sent on 27/07/2015
> > 
> >  drivers/gpu/drm/i915/intel_dp.c | 6 ++++--
> >  1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 18bcfbe..2a8b47e 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -6174,8 +6174,9 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> >  	intel_encoder = &intel_dig_port->base;
> >  	encoder = &intel_encoder->base;
> >  
> > -	drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
> > -			 DRM_MODE_ENCODER_TMDS);
> > +	if (drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
> > +			     DRM_MODE_ENCODER_TMDS))
> > +		goto err_encoder_init;
> >  
> >  	intel_encoder->compute_config = intel_dp_compute_config;
> >  	intel_encoder->disable = intel_disable_dp;
> > @@ -6224,6 +6225,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> >  
> >  err_init_connector:
> >  	drm_encoder_cleanup(encoder);
> > +err_encoder_init:
> >  	kfree(intel_connector);
> >  err_connector_alloc:
> >  	kfree(intel_dig_port);
> > -- 
> > 1.9.1
> > 
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH RESEND 2/3] drm/i915: check for return value
  2015-12-09 11:39     ` Sudip Mukherjee
@ 2015-12-09 13:33       ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2015-12-09 13:33 UTC (permalink / raw)
  To: Sudip Mukherjee
  Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Daniel Vetter

On Wed, Dec 09, 2015 at 05:09:12PM +0530, Sudip Mukherjee wrote:
> On Thu, Oct 08, 2015 at 04:29:31PM +0200, Daniel Vetter wrote:
> > On Thu, Oct 08, 2015 at 07:28:00PM +0530, Sudip Mukherjee wrote:
> > > We were not checking the return value of drm_encoder_init() which can
> > > fail. And if it fails then we will be working with an uninitialized
> > > encoder.
> > > 
> > > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > > Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> > 
> > Queued for -next, thanks for the patch.
> > -Daniel
> 
> Hi Daniel,
> It is still not there in linux-next. Still applies cleanly on
> next-20151209.

Sorry, this must have fallen through the cracks somehow. Applied now for
real.

Thanks, Daniel

> 
> regards
> sudip
> 
> > 
> > > ---
> > > 
> > > Sent on 27/07/2015
> > > 
> > >  drivers/gpu/drm/i915/intel_dp.c | 6 ++++--
> > >  1 file changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index 18bcfbe..2a8b47e 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -6174,8 +6174,9 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> > >  	intel_encoder = &intel_dig_port->base;
> > >  	encoder = &intel_encoder->base;
> > >  
> > > -	drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
> > > -			 DRM_MODE_ENCODER_TMDS);
> > > +	if (drm_encoder_init(dev, &intel_encoder->base, &intel_dp_enc_funcs,
> > > +			     DRM_MODE_ENCODER_TMDS))
> > > +		goto err_encoder_init;
> > >  
> > >  	intel_encoder->compute_config = intel_dp_compute_config;
> > >  	intel_encoder->disable = intel_disable_dp;
> > > @@ -6224,6 +6225,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> > >  
> > >  err_init_connector:
> > >  	drm_encoder_cleanup(encoder);
> > > +err_encoder_init:
> > >  	kfree(intel_connector);
> > >  err_connector_alloc:
> > >  	kfree(intel_dig_port);
> > > -- 
> > > 1.9.1
> > > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-09 13:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-08 13:57 [PATCH RESEND 1/3] drm/i915: use error path Sudip Mukherjee
2015-10-08 13:58 ` [PATCH RESEND 2/3] drm/i915: check for return value Sudip Mukherjee
2015-10-08 14:29   ` Daniel Vetter
2015-12-09 11:39     ` Sudip Mukherjee
2015-12-09 13:33       ` Daniel Vetter
2015-10-08 14:30 ` [PATCH RESEND 1/3] drm/i915: use error path Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox