From mboxrd@z Thu Jan  1 00:00:00 1970
Content-Type: multipart/mixed; boundary="===============1692079623330902100=="
MIME-Version: 1.0
From: Mark Wielaard <mjw@redhat.com>
To: elfutils-devel@lists.fedorahosted.org
Subject: Re: [PATCH] Prepare 0.167 release.
Date: Tue, 09 Aug 2016 22:58:35 +0200
Message-ID: <20160809205835.GO2508@stream>
In-Reply-To: 874m6ukvbt.fsf@oracle.com

--===============1692079623330902100==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable

Hi Jose,

On Tue, Aug 09, 2016 at 01:58:30PM +0200, Jose E. Marchesi wrote:
> Ok for master?

OK with the following nitpicks below.

> commit c7e007a9f3bd550a2f00e41f303f23175af0dcca
> Author: Jose E. Marchesi <jose.marchesi@oracle.com>
> Date:   Tue Aug 9 04:30:37 2016 -0700
> =

>     sparc: fix the printing of hw capabilities object attributes.
>     =

>     The GNU_Sparc_HWCAPS and GNU_Sparc_HWCAPS2 object attributes comprise=
 a
>     set of hardware capabilities that may (or not) be present in the targ=
et
>     machine for which the object was compiled.  This patch adds the suppo=
rt
>     for printing a nicely formatted comma-separated list with the selected
>     hw capabilities.
>     =

>     Signed-off-by: Jose E. Marchesi <jose.marchesi@oracle.com>
> =


> @@ -41,33 +41,62 @@ sparc_check_object_attribute (Ebl *ebl __attribute__ =
((unused)),
>  			      const char *vendor, int tag, uint64_t value,
>  			      const char **tag_name, const char **value_name)
>  {
> +  static const char *hwcaps[32] =3D
> +    {
> +     "mul32", "div32", "fsmuld", "v8plus", "popc", "vis", "vis2", "asi_b=
lk_init", "fmaf",
> +      "vis3", "hpc", "random", "trans", "fjfmau", "ima",
> +      "asi_cache_sparing", "aes", "des", "kasumi", "camellia", "md5", "s=
ha1",
> +      "sha256", "sha512", "mpmul", "mont", "pause", "cbcond", "crc32c", =
"resv30",
> +      "resv31"
> +    };
> +
> +  =

> +  static const char *hwcaps2[32] =3D
> +    {
> +      "fjathplus", "vis3b", "adp", "sparc5", "mwait", "xmpmul", "xmont",=
 "nsec",
> +      "resv8", "resv9" , "resv10", "resv11", "fjathhpc", "fjdes", "fjaes=
", "resv15",
> +      "resv16", "resv17", "resv18", "resv19", "resv20", "resv21", "resv2=
2", "resv23",
> +      "resv24", "resv25", "resv26", "resv27", "resv28", "resv29", "resv3=
0", "resv31",
> +    };

If possible try to keep lines < 80 chars please.

> +          char *s =3D name;
> +          for (cap =3D 0; cap < 32; cap++)
> +            if (value & (1 << cap))

Use (1U << cap) since signed left shift is undefined if you move 1 into
the signed bit.

> +              {
> +                if (*s !=3D '\0')
> +                  s =3D strcat (s, ",");
> +                s =3D strcat (s, caps[cap] ? caps[cap] : "unknown");

Given the arrays above "unknown" can never happen?

> diff --git a/tests/testfilesparc64attrs.o.bz2 b/tests/testfilesparc64attr=
s.o.bz2
> new file mode 100644
> index 0000000..7be7f88
> Binary files /dev/null and b/tests/testfilesparc64attrs.o.bz2 differ

I trust this is as described. But if you could use git format-patch next
time that would be appreciated.

Thanks,

Mark
--===============1692079623330902100==--