| Message ID | 1396691102-22904-10-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/05/2014 02:44 AM, Daniel Vetter wrote: > The outer if already checks for data != 0, so it can't really be > 0. Hence remove it. > > Now I don't have specs or anything for this beast, so I have no > idea whether this was actually intended or whether the logic > should be different. At least the code still seems to be doing > something useful. > > Spotted by coverity. > > Cc: Dave Airlie <airlied@redhat.com> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/ast/ast_post.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c > index 977cfb35837a..6263116054b6 100644 > --- a/drivers/gpu/drm/ast/ast_post.c > +++ b/drivers/gpu/drm/ast/ast_post.c > @@ -572,8 +572,6 @@ static u32 cbr_scan2(struct ast_private *ast) > for (loop = 0; loop < CBR_PASSNUM2; loop++) { > if ((data = cbr_test2(ast)) != 0) { > data2 &= data; > - if (!data) > - return 0; That feels like a typo... was that supposed to be 'if (!data2)'? > break; > } > } >
On Mon, Apr 7, 2014 at 5:28 PM, Ian Romanick <idr@freedesktop.org> wrote: > On 04/05/2014 02:44 AM, Daniel Vetter wrote: >> The outer if already checks for data != 0, so it can't really be >> 0. Hence remove it. >> >> Now I don't have specs or anything for this beast, so I have no >> idea whether this was actually intended or whether the logic >> should be different. At least the code still seems to be doing >> something useful. >> >> Spotted by coverity. >> >> Cc: Dave Airlie <airlied@redhat.com> >> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> >> --- >> drivers/gpu/drm/ast/ast_post.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c >> index 977cfb35837a..6263116054b6 100644 >> --- a/drivers/gpu/drm/ast/ast_post.c >> +++ b/drivers/gpu/drm/ast/ast_post.c >> @@ -572,8 +572,6 @@ static u32 cbr_scan2(struct ast_private *ast) >> for (loop = 0; loop < CBR_PASSNUM2; loop++) { >> if ((data = cbr_test2(ast)) != 0) { >> data2 &= data; >> - if (!data) >> - return 0; > > That feels like a typo... was that supposed to be 'if (!data2)'? Yeah this one really needs a close look, since I have no idea what's actually intended behaviour. The patch just removes the dead code as it is now, and the double-loop still makes some sense imo after this change. But I really don't know the spec for this hw. -Daniel
diff --git a/drivers/gpu/drm/ast/ast_post.c b/drivers/gpu/drm/ast/ast_post.c index 977cfb35837a..6263116054b6 100644 --- a/drivers/gpu/drm/ast/ast_post.c +++ b/drivers/gpu/drm/ast/ast_post.c @@ -572,8 +572,6 @@ static u32 cbr_scan2(struct ast_private *ast) for (loop = 0; loop < CBR_PASSNUM2; loop++) { if ((data = cbr_test2(ast)) != 0) { data2 &= data; - if (!data) - return 0; break; } }
The outer if already checks for data != 0, so it can't really be 0. Hence remove it. Now I don't have specs or anything for this beast, so I have no idea whether this was actually intended or whether the logic should be different. At least the code still seems to be doing something useful. Spotted by coverity. Cc: Dave Airlie <airlied@redhat.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/ast/ast_post.c | 2 -- 1 file changed, 2 deletions(-)