| Message ID | 1404564682-21983-1-git-send-email-rickard_strandqvist@spectrumdigital.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sat 2014-07-05 14:51:22, Rickard Strandqvist wrote: > From: Rickard Strandqvist <rickard.strandqvist@sonymobile.com> > > Variable ar assigned a value that is never used. > I have also removed all the code that thereby serves no purpose. > > This was found using a static code analysis program called cppcheck Are you sure this is right fix? Should we be returning the error when there's error?
On Tue 2014-07-22 11:16:58, Rickard Strandqvist wrote: > Hi > > Sure, I agree. But as I thought that I would not change > currentfunctionality, I would increase the chance that the patches were > included. And it would of course also clarify this type of problem. I'm trying to say that getting rid of the variables without changing functionality might be wrong thing to do, for example in this case. It looks like error handing is missing by mistake, and you are removing traces saying that error handing is required here. Dunno. Perhaps don't push patches where solution is not obvious? Or push them but mark the place with fixme? Pavel
Hi There is a lot in what you say. In the beginning I send in a remark to the maintainer. It happened absolutely nothing. When I send out a patch that code is not used, then this code will be reviewed. Which is the main purpose! However, agree that this was perhaps an unusually clear solution. I submit such a patch now. Kind regards Rickard Strandqvist 2014-07-22 11:27 GMT+02:00 Pavel Machek <pavel@ucw.cz>: > On Tue 2014-07-22 11:16:58, Rickard Strandqvist wrote: >> Hi >> >> Sure, I agree. But as I thought that I would not change >> currentfunctionality, I would increase the chance that the patches were >> included. And it would of course also clarify this type of problem. > > I'm trying to say that getting rid of the variables without changing > functionality might be wrong thing to do, for example in this case. It > looks like error handing is missing by mistake, and you are removing > traces saying that error handing is required here. > > Dunno. Perhaps don't push patches where solution is not obvious? > > Or push them but mark the place with fixme? > > Pavel > > -- > (english) http://www.livejournal.com/~pavelmachek > (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/video/fbdev/uvesafb.c b/drivers/video/fbdev/uvesafb.c index 509d452..50086b9 100644 --- a/drivers/video/fbdev/uvesafb.c +++ b/drivers/video/fbdev/uvesafb.c @@ -555,12 +555,12 @@ static int uvesafb_vbe_getmodes(struct uvesafb_ktask *task, static int uvesafb_vbe_getpmi(struct uvesafb_ktask *task, struct uvesafb_par *par) { - int i, err; + int i; uvesafb_reset(task); task->t.regs.eax = 0x4f0a; task->t.regs.ebx = 0x0; - err = uvesafb_exec(task); + uvesafb_exec(task); if ((task->t.regs.eax & 0xffff) != 0x4f || task->t.regs.es < 0xc000) { par->pmi_setpal = par->ypan = 0;