On Thu, 2020-12-17 at 09:27 -0800, Linus Torvalds wrote:
> On Thu, Dec 17, 2020 at 8:25 AM Josh Poimboeuf <jpoim...@redhat.com> wrote:
> >
> > Oh yeah, I forgot about that. That would be another option if my patch
> > doesn't work out.
>
> Well, one option is to just say "ok, we know gcc generates horrible
> code that falls through to another function in a situation that we
> claim is unreachable, so let's not claim it is unreachable".
>
> IOW, the problem here is that the compiler fundamentally isn't smart
> enough to see that something is unreachable, and the "unreachable()"
> annotation we did didn't actually really cause any code that makes it
> so. So we basically have code that _if_ we ever change it, it will
> simply be wrong, and we'll never see any warnings about it but it will
> fall through to nonsensical code.
>
> So maybe the option here is simply "objtool was right before, the
> unreachable() is fragile and wrong".
>
> We can easily write that case statement in a way that actually makes
> the compiler generate better code and avoids the issue by just making
> case 0x00 also be the default case.
>
> So I think I'll just apply this patch instead.
Using default somewhere other than the bottom of a switch/case block
isn't common/pretty. It's easy to visually skip/glaze over.
Perhaps reorder the block.
Maybe add static to the const arrays too.
---
drivers/gpu/drm/drm_edid.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 74f5a3197214..53b7bb281edb 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3089,8 +3089,8 @@ static int drm_cvt_modes(struct drm_connector *connector,
struct drm_display_mode *newmode;
struct drm_device *dev = connector->dev;
struct cvt_timing *cvt;
- const int rates[] = { 60, 85, 75, 60, 50 };
- const u8 empty[3] = { 0, 0, 0 };
+ static const int rates[] = { 60, 85, 75, 60, 50 };
+ static const u8 empty[3] = { 0, 0, 0 };
for (i = 0; i < 4; i++) {
int width, height;
@@ -3102,20 +3102,18 @@ static int drm_cvt_modes(struct drm_connector
*connector,
height = (cvt->code[0] + ((cvt->code[1] & 0xf0) << 4) + 1) * 2;
switch (cvt->code[1] & 0x0c) {
- case 0x00:
- width = height * 4 / 3;
- break;
- case 0x04:
- width = height * 16 / 9;
+ case 0x0c:
+ width = height * 15 / 9;
break;
case 0x08:
width = height * 16 / 10;
break;
- case 0x0c:
- width = height * 15 / 9;
+ case 0x04:
+ width = height * 16 / 9;
break;
default:
- unreachable();
+ width = height * 4 / 3;
+ break;
}
for (j = 1; j < 5; j++) {
