diff mbox

drm/i915: fix itnull.cocci warnings (fwd)

Message ID alpine.DEB.2.10.1601181647260.2520@hadrien (mailing list archive)
State New, archived
Headers show

Commit Message

Julia Lawall Jan. 18, 2016, 3:49 p.m. UTC
List_for_each entry binds its first argument to an offset from the list
pointer, so this should not be NULL.

Generated by: scripts/coccinelle/iterators/itnull.cocci

Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
---

Please take the patch only if it's a positive warning. Thanks!

 intel_display.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Vetter Jan. 18, 2016, 5:20 p.m. UTC | #1
On Mon, Jan 18, 2016 at 04:49:06PM +0100, Julia Lawall wrote:
> List_for_each entry binds its first argument to an offset from the list
> pointer, so this should not be NULL.
> 
> Generated by: scripts/coccinelle/iterators/itnull.cocci
> 
> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> ---
> 
> Please take the patch only if it's a positive warning. Thanks!

Against which tree is this? I can't find this anywhere like that ...
-Daniel

> 
>  intel_display.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -16498,7 +16498,7 @@ void intel_modeset_preclose(struct drm_d
>  		struct intel_flip_work *work;
> 
>  		list_for_each_entry(work, &crtc->flip_work, head) {
> -			if (work && work->event &&
> +			if (work->event &&
>  			    work->event->base.file_priv == file) {
>  				kfree(work->event);
>  				work->event = NULL;
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Eric Engestrom Jan. 18, 2016, 5:42 p.m. UTC | #2
I expect this is the script she mentions:
https://github.com/coccinelle/coccinellery/blob/master/itnull/itnull.cocci

Julia is one of the authors of Coccinelle, and the author of that script :)


On 18/01/16 17:20, Daniel Vetter wrote:
> On Mon, Jan 18, 2016 at 04:49:06PM +0100, Julia Lawall wrote:
>> List_for_each entry binds its first argument to an offset from the list
>> pointer, so this should not be NULL.
>>
>> Generated by: scripts/coccinelle/iterators/itnull.cocci
>>
>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>> ---
>>
>> Please take the patch only if it's a positive warning. Thanks!
> 
> Against which tree is this? I can't find this anywhere like that ...
> -Daniel
> 
>>
>>  intel_display.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -16498,7 +16498,7 @@ void intel_modeset_preclose(struct drm_d
>>  		struct intel_flip_work *work;
>>
>>  		list_for_each_entry(work, &crtc->flip_work, head) {
>> -			if (work && work->event &&
>> +			if (work->event &&
>>  			    work->event->base.file_priv == file) {
>>  				kfree(work->event);
>>  				work->event = NULL;
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
Daniel Vetter Jan. 18, 2016, 6:02 p.m. UTC | #3
On Mon, Jan 18, 2016 at 05:42:24PM +0000, Eric Engestrom wrote:
> I expect this is the script she mentions:
> https://github.com/coccinelle/coccinellery/blob/master/itnull/itnull.cocci
> 
> Julia is one of the authors of Coccinelle, and the author of that script :)

I get how these patches get created, I just can't find a tree anywhere
where this applies. So I wonder what it was generated against ...
-Daniel

> 
> 
> On 18/01/16 17:20, Daniel Vetter wrote:
> > On Mon, Jan 18, 2016 at 04:49:06PM +0100, Julia Lawall wrote:
> >> List_for_each entry binds its first argument to an offset from the list
> >> pointer, so this should not be NULL.
> >>
> >> Generated by: scripts/coccinelle/iterators/itnull.cocci
> >>
> >> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> >> ---
> >>
> >> Please take the patch only if it's a positive warning. Thanks!
> > 
> > Against which tree is this? I can't find this anywhere like that ...
> > -Daniel
> > 
> >>
> >>  intel_display.c |    2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -16498,7 +16498,7 @@ void intel_modeset_preclose(struct drm_d
> >>  		struct intel_flip_work *work;
> >>
> >>  		list_for_each_entry(work, &crtc->flip_work, head) {
> >> -			if (work && work->event &&
> >> +			if (work->event &&
> >>  			    work->event->base.file_priv == file) {
> >>  				kfree(work->event);
> >>  				work->event = NULL;
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
Julia Lawall Jan. 18, 2016, 8:20 p.m. UTC | #4
On Mon, 18 Jan 2016, Daniel Vetter wrote:

> On Mon, Jan 18, 2016 at 04:49:06PM +0100, Julia Lawall wrote:
> > List_for_each entry binds its first argument to an offset from the list
> > pointer, so this should not be NULL.
> > 
> > Generated by: scripts/coccinelle/iterators/itnull.cocci
> > 
> > Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
> > ---
> > 
> > Please take the patch only if it's a positive warning. Thanks!
> 
> Against which tree is this? I can't find this anywhere like that ...

I don't know.  It may be against a submitted patch.

In case it turns out to be useful, I forgot:

Signed-off-by: Julia Lawall <julia.lawall@lip6.fr>


> -Daniel
> 
> > 
> >  intel_display.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -16498,7 +16498,7 @@ void intel_modeset_preclose(struct drm_d
> >  		struct intel_flip_work *work;
> > 
> >  		list_for_each_entry(work, &crtc->flip_work, head) {
> > -			if (work && work->event &&
> > +			if (work->event &&
> >  			    work->event->base.file_priv == file) {
> >  				kfree(work->event);
> >  				work->event = NULL;
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
>
Maarten Lankhorst Jan. 20, 2016, 10:05 a.m. UTC | #5
Op 18-01-16 om 18:20 schreef Daniel Vetter:
> On Mon, Jan 18, 2016 at 04:49:06PM +0100, Julia Lawall wrote:
>> List_for_each entry binds its first argument to an offset from the list
>> pointer, so this should not be NULL.
>>
>> Generated by: scripts/coccinelle/iterators/itnull.cocci
>>
>> Signed-off-by: Fengguang Wu <fengguang.wu@intel.com>
>> ---
>>
>> Please take the patch only if it's a positive warning. Thanks!
> Against which tree is this? I can't find this anywhere like that ...
>
Looks to like it happens in my tree.

drm/i915: Convert flip_work to a list.

~Maarten
diff mbox

Patch

--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -16498,7 +16498,7 @@  void intel_modeset_preclose(struct drm_d
 		struct intel_flip_work *work;

 		list_for_each_entry(work, &crtc->flip_work, head) {
-			if (work && work->event &&
+			if (work->event &&
 			    work->event->base.file_priv == file) {
 				kfree(work->event);
 				work->event = NULL;