Recent change to display_arc_attribute

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Recent change to display_arc_attribute

Jeff Law

Recent changes to display_arc_attribute are causing significant failures
to build with modern compilers due to introduction of a switch case
fallthru.

The code in question:

     case Tag_ARC_CPU_variation:
       val = read_uleb128 (p, &len, end);
       p += len;
       printf ("  Tag_ARC_CPU_variation: ");
       switch (val)
         {
         default:
           if (val > 0 && val < 16)
             {
               printf ("Core%d\n", val);
               break;
             }
         case 0:
           printf (_("Absent\n"));
           break;
         }
       break;

Note the fallthru from the default to case 0.


I'm not at all familiar with the ARC port and what we want to do here.
But the attached seems fairly benign in that if someone added an out of
range variation we'd flag it as "Unknown" rather than "Absent".

Thoughts?

Jeff

diff --git a/binutils/readelf.c b/binutils/readelf.c
index 16eb866..39bc88f 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -13586,10 +13586,11 @@ display_arc_attribute (unsigned char * p,
  {
  default:
   if (val > 0 && val < 16)
-    {
       printf ("Core%d\n", val);
-      break;
-    }
+  else
+      printf ("Unknown\n");
+  break;
+
  case 0:
   printf (_("Absent\n"));
   break;
Reply | Threaded
Open this post in threaded view
|

Re: Recent change to display_arc_attribute

Claudiu Zissulescu-2
Hi Jeff,

It looks alright from my side. Sorry for this overlooking. I'm also
preparing a patch to add the fall through comments.

Thank you,
Claudiu

On Mon, May 15, 2017 at 4:53 PM, Jeff Law <[hidden email]> wrote:

>
> Recent changes to display_arc_attribute are causing significant failures to
> build with modern compilers due to introduction of a switch case fallthru.
>
> The code in question:
>
>     case Tag_ARC_CPU_variation:
>       val = read_uleb128 (p, &len, end);
>       p += len;
>       printf ("  Tag_ARC_CPU_variation: ");
>       switch (val)
>         {
>         default:
>           if (val > 0 && val < 16)
>             {
>               printf ("Core%d\n", val);
>               break;
>             }
>         case 0:
>           printf (_("Absent\n"));
>           break;
>         }
>       break;
>
> Note the fallthru from the default to case 0.
>
>
> I'm not at all familiar with the ARC port and what we want to do here. But
> the attached seems fairly benign in that if someone added an out of range
> variation we'd flag it as "Unknown" rather than "Absent".
>
> Thoughts?
>
> Jeff
>
> diff --git a/binutils/readelf.c b/binutils/readelf.c
> index 16eb866..39bc88f 100644
> --- a/binutils/readelf.c
> +++ b/binutils/readelf.c
> @@ -13586,10 +13586,11 @@ display_arc_attribute (unsigned char * p,
>         {
>         default:
>           if (val > 0 && val < 16)
> -           {
>               printf ("Core%d\n", val);
> -             break;
> -           }
> +         else
> +             printf ("Unknown\n");
> +         break;
> +
>         case 0:
>           printf (_("Absent\n"));
>           break;
>
Reply | Threaded
Open this post in threaded view
|

Re: Recent change to display_arc_attribute

Jeff Law
On 05/15/2017 10:36 AM, Claudiu Zissulescu wrote:
> Hi Jeff,
>
> It looks alright from my side. Sorry for this overlooking. I'm also
> preparing a patch to add the fall through comments.
Thanks.  Installed.

Jeff