Re: [PATCH] nis: Fix nis_print_directory

[Arjun Shankar <arjun@redhat.com>]

Hi Adhemerval,

> Remove implicit conversion from enumeration type 'zotypes' to different
> type 'nstype'.
>
> Checked on x86_64-linux-gnu.

Overall, the patch looks good to me and with the patch in place, glibc
builds cleanly with -Wenum-conversion. One question below; but even
without that change, I'm okay with this patch. It fixes a bug and is
an improvement.

Reviewed-by: Arjun Shankar <arjun@redhat.com>

> ---
>  nis/nis_print.c | 59 +++++++++++++++++++++++--------------------------
>  1 file changed, 28 insertions(+), 31 deletions(-)
>
> diff --git a/nis/nis_print.c b/nis/nis_print.c
> index d0efd98e86..2708c29ef0 100644
> --- a/nis/nis_print.c
> +++ b/nis/nis_print.c
> @@ -52,6 +52,32 @@ nis_nstype2str (const nstype type)
>      }
>  }
>
> +static const char *
> +nis_objtype (const zotypes type)
> +{
> +  switch (type)
> +    {
> +    case NIS_BOGUS_OBJ:
> +      return _("BOGUS OBJECT");
> +    case NIS_NO_OBJ:
> +      return _("NO OBJECT");
> +    case NIS_DIRECTORY_OBJ:
> +      return _("DIRECTORY");
> +    case NIS_GROUP_OBJ:
> +      return _("GROUP");
> +    case NIS_TABLE_OBJ:
> +      return _("TABLE");
> +    case NIS_ENTRY_OBJ:
> +      return _("ENTRY");
> +    case NIS_LINK_OBJ:
> +      return _("LINK");
> +    case NIS_PRIVATE_OBJ:
> +      return _("PRIVATE\n");
> +    default:
> +      return _("(Unknown object");
> +    }
> +}
> +
>  static void
>  print_ttl (const uint32_t ttl)
>  {

New function to return strings corresponding to zotype. Translating
these strings makes sense. I'm wondering if they should be in
title-case instead of all-caps? Looks like "Table", "Directory" etc.
are being printed elsewhere as part of longer translatable strings
already.

> @@ -103,36 +129,7 @@ print_flags (const unsigned int flags)
>  static void
>  nis_print_objtype (enum zotypes type)
>  {
> -  switch (type)
> -    {
> -    case NIS_BOGUS_OBJ:
> -      fputs (_("BOGUS OBJECT\n"), stdout);
> -      break;
> -    case NIS_NO_OBJ:
> -      fputs (_("NO OBJECT\n"), stdout);
> -      break;
> -    case NIS_DIRECTORY_OBJ:
> -      fputs (_("DIRECTORY\n"), stdout);
> -      break;
> -    case NIS_GROUP_OBJ:
> -      fputs (_("GROUP\n"), stdout);
> -      break;
> -    case NIS_TABLE_OBJ:
> -      fputs (_("TABLE\n"), stdout);
> -      break;
> -    case NIS_ENTRY_OBJ:
> -      fputs (_("ENTRY\n"), stdout);
> -      break;
> -    case NIS_LINK_OBJ:
> -      fputs (_("LINK\n"), stdout);
> -      break;
> -    case NIS_PRIVATE_OBJ:
> -      fputs (_("PRIVATE\n"), stdout);
> -      break;
> -    default:
> -      fputs (_("(Unknown object)\n"), stdout);
> -      break;
> -    }
> +  printf ("%s\n", nis_objtype (type));
>  }
>
>  void

OK. Get the string from the new function instead of duplicating the
switch statement.

> @@ -236,7 +233,7 @@ nis_print_directory (const directory_obj *dir)
>        for (i = 0; i < dir->do_armask.do_armask_len; i++)
>         {
>           nis_print_rights (ptr->oa_rights);
> -         printf (_("\tType         : %s\n"), nis_nstype2str (ptr->oa_otype));
> +         printf (_("\tType         : %s\n"), nis_objtype (ptr->oa_otype));
>           fputs (_("\tAccess rights: "), stdout);
>           nis_print_rights (ptr->oa_rights);
>           fputs ("\n", stdout);

OK. This was quite wrong earlier, printing strings corresponding to
the wrong enum. Fixed now.

> --
> 2.34.1
>