Re: [dmidecode] [patch #8989] Output "Invalid entry length.. Fixed up" warning to stderr

[Kevin Bowling]

Thanks, this looks good to me.

Regards,
Kevin

On Mon, Jul 4, 2016 at 3:12 AM, Jean Delvare <address@hidden> wrote:

> Hi Kevin,
>
> On Thu, 28 Apr 2016 18:56:26 -0700, Kevin Bowling wrote:
> > dmidecode 3.0 breaks DMI values of some tools like SaltStack that read
> > stdout for specific values to populate data structures about a host on
> some
> > hardware because of a warning like "Invalid entry length (16). Fixed up
> to
> > 11."
> >
> > My proposed fix is to simply move it to stderr
> > http://savannah.nongnu.org/patch/?8989
>
> I've generalized it after seeing another related bug report. Can you
> try the following patch?
>
> From: Jean Delvare <address@hidden>
> Subject: dmidecode: Move error messages to stderr
>
> Consistently write error messages to stderr, to avoid messing up the
> output of "dmidecode -s". Based on preliminary patches by
> Kevin Bowling and Simon Rettberg.
>
> Fixes bug #47274:
> https://savannah.nongnu.org/bugs/?47274
> Fixes bug #48158:
> https://savannah.nongnu.org/bugs/?48158
> Supersedes patch #8989:
> https://savannah.nongnu.org/patch/?8989
> ---
>  dmidecode.c |   37 ++++++++++++++++++++++++++-----------
>  1 file changed, 26 insertions(+), 11 deletions(-)
>
> --- dmidecode.orig/dmidecode.c  2016-07-04 11:36:36.961481311 +0200
> +++ dmidecode/dmidecode.c       2016-07-04 12:06:51.529344282 +0200
> @@ -2958,7 +2958,8 @@ static void dmi_fixup_type_34(struct dmi
>          && is_printable(p + 0x0B, 0x10 - 0x0B))
>         {
>                 if (!(opt.flags & FLAG_QUIET) && display)
> -                       printf("Invalid entry length (%u). Fixed up to
> %u.\n",
> +                       fprintf(stderr,
> +                               "Invalid entry length (%u). Fixed up to
> %u.\n",
>                                 0x10, 0x0B);
>                 h->length = 0x0B;
>         }
> @@ -4427,9 +4428,14 @@ static void dmi_table_decode(u8 *buf, u3
>                  */
>                 if (h.length < 4)
>                 {
> -                       printf("Invalid entry length (%u). DMI table is "
> -                              "broken! Stop.\n\n", (unsigned
> int)h.length);
> -                       opt.flags |= FLAG_QUIET;
> +                       if (!(opt.flags & FLAG_QUIET))
> +                       {
> +                               fprintf(stderr,
> +                                       "Invalid entry length (%u). DMI
> table "
> +                                       "is broken! Stop.\n\n",
> +                                       (unsigned int)h.length);
> +                               opt.flags |= FLAG_QUIET;
> +                       }
>                         break;
>                 }
>
> @@ -4490,11 +4496,11 @@ static void dmi_table_decode(u8 *buf, u3
>         if (!(opt.flags & FLAG_QUIET))
>         {
>                 if (num && i != num)
> -                       printf("Wrong DMI structures count: %d announced, "
> +                       fprintf(stderr, "Wrong DMI structures count: %d
> announced, "
>                                 "only %d decoded.\n", num, i);
>                 if ((unsigned long)(data - buf) > len
>                  || (num && (unsigned long)(data - buf) < len))
> -                       printf("Wrong DMI structures length: %u bytes "
> +                       fprintf(stderr, "Wrong DMI structures length: %u
> bytes "
>                                 "announced, structures occupy %lu
> bytes.\n",
>                                 len, (unsigned long)(data - buf));
>         }
> @@ -4539,7 +4545,7 @@ static void dmi_table(off_t base, u32 le
>                 buf = read_file(&size, devmem);
>                 if (!(opt.flags & FLAG_QUIET) && num && size !=
> (size_t)len)
>                 {
> -                       printf("Wrong DMI structures length: %u bytes "
> +                       fprintf(stderr, "Wrong DMI structures length: %u
> bytes "
>                                 "announced, only %lu bytes available.\n",
>                                 len, (unsigned long)size);
>                 }
> @@ -4652,14 +4658,16 @@ static int smbios_decode(u8 *buf, const
>                 case 0x021F:
>                 case 0x0221:
>                         if (!(opt.flags & FLAG_QUIET))
> -                               printf("SMBIOS version fixup (2.%d ->
> 2.%d).\n",
> -                                      ver & 0xFF, 3);
> +                               fprintf(stderr,
> +                                       "SMBIOS version fixup (2.%d ->
> 2.%d).\n",
> +                                       ver & 0xFF, 3);
>                         ver = 0x0203;
>                         break;
>                 case 0x0233:
>                         if (!(opt.flags & FLAG_QUIET))
> -                               printf("SMBIOS version fixup (2.%d ->
> 2.%d).\n",
> -                                      51, 6);
> +                               fprintf(stderr,
> +                                       "SMBIOS version fixup (2.%d ->
> 2.%d).\n",
> +                                       51, 6);
>                         ver = 0x0206;
>                         break;
>         }
> @@ -4771,6 +4779,13 @@ int main(int argc, char * const argv[])
>         int efi;
>         u8 *buf;
>
> +       /*
> +        * We don't want stdout and stderr to be mixed up if both are
> +        * redirected to the same file.
> +        */
> +       setlinebuf(stdout);
> +       setlinebuf(stderr);
> +
>         if (sizeof(u8) != 1 || sizeof(u16) != 2 || sizeof(u32) != 4 ||
> '\0' != 0)
>         {
>                 fprintf(stderr, "%s: compiler incompatibility\n", argv[0]);
>
>
> --
> Jean Delvare
> SUSE L3 Support
>