Re: [PATCH v3] grub-install: locale depends on nls

2018-04-17 Thread Daniel Kiper
On Fri, Apr 13, 2018 at 11:36:49PM +0200, Olaf Hering wrote:
> With --disable-nls no locales exist.
>
> Avoid runtime error by moving code that copies locales into its own
> function. Return early in case nls was disabled. That way the compiler
> will throw away unreachable code, no need to put preprocessor
> conditionals everywhere to avoid warnings about unused code.
>
> Fix memleak by freeing dstf.

s/dstf/srcf and dstf/?

> Convert tabs to spaces in moved code.
>
> Signed-off-by: Olaf Hering 

Otherwise LGTM. If there are no objections I will commit
this patch in a week or so.

Daniel

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH v3] grub-install: locale depends on nls

2018-04-13 Thread Olaf Hering
With --disable-nls no locales exist.

Avoid runtime error by moving code that copies locales into its own
function. Return early in case nls was disabled. That way the compiler
will throw away unreachable code, no need to put preprocessor
conditionals everywhere to avoid warnings about unused code.

Fix memleak by freeing dstf.
Convert tabs to spaces in moved code.

Signed-off-by: Olaf Hering 
---
 util/grub-install-common.c | 108 +
 1 file changed, 59 insertions(+), 49 deletions(-)

diff --git a/util/grub-install-common.c b/util/grub-install-common.c
index 9e3e358c9..746d5be6b 100644
--- a/util/grub-install-common.c
+++ b/util/grub-install-common.c
@@ -592,6 +592,7 @@ copy_all (const char *srcd,
   grub_util_fd_closedir (d);
 }
 
+#if !(defined (GRUB_UTIL) && defined(ENABLE_NLS) && ENABLE_NLS)
 static const char *
 get_localedir (void)
 {
@@ -646,6 +647,61 @@ copy_locales (const char *dstd)
 }
   grub_util_fd_closedir (d);
 }
+#endif
+
+static void
+grub_install_copy_nls(const char *src __attribute__ ((unused)),
+  const char *dst __attribute__ ((unused)))
+{
+#if !(defined (GRUB_UTIL) && defined(ENABLE_NLS) && ENABLE_NLS)
+  char *dst_locale;
+
+  dst_locale = grub_util_path_concat (2, dst, "locale");
+  grub_install_mkdir_p (dst_locale);
+  clean_grub_dir (dst_locale);
+
+  if (install_locales.is_default)
+{
+  char *srcd = grub_util_path_concat (2, src, "po");
+  copy_by_ext (srcd, dst_locale, ".mo", 0);
+  copy_locales (dst_locale);
+  free (srcd);
+}
+  else
+{
+  size_t i;
+  const char *locale_dir = get_localedir ();
+
+  for (i = 0; i < install_locales.n_entries; i++)
+  {
+char *srcf = grub_util_path_concat_ext (3, src, "po",
+install_locales.entries[i],
+".mo");
+char *dstf = grub_util_path_concat_ext (2, dst_locale,
+install_locales.entries[i],
+".mo");
+if (grub_install_compress_file (srcf, dstf, 0))
+  {
+free (srcf);
+free (dstf);
+continue;
+  }
+free (srcf);
+srcf = grub_util_path_concat_ext (4, locale_dir,
+  install_locales.entries[i],
+  "LC_MESSAGES", PACKAGE, ".mo");
+if (grub_install_compress_file (srcf, dstf, 0) == 0)
+  {
+grub_util_error (_("cannot find locale `%s'"),
+ install_locales.entries[i]);
+  }
+free (srcf);
+free (dstf);
+  }
+}
+  free (dst_locale);
+#endif
+}
 
 static struct
 {
@@ -731,7 +787,7 @@ grub_install_copy_files (const char *src,
 const char *dst,
 enum grub_install_plat platid)
 {
-  char *dst_platform, *dst_locale, *dst_fonts;
+  char *dst_platform, *dst_fonts;
   const char *pkgdatadir = grub_util_get_pkgdatadir ();
   char *themes_dir;
 
@@ -742,13 +798,12 @@ grub_install_copy_files (const char *src,
 dst_platform = grub_util_path_concat (2, dst, platform);
 free (platform);
   }
-  dst_locale = grub_util_path_concat (2, dst, "locale");
   dst_fonts = grub_util_path_concat (2, dst, "fonts");
   grub_install_mkdir_p (dst_platform);
-  grub_install_mkdir_p (dst_locale);
   clean_grub_dir (dst);
   clean_grub_dir (dst_platform);
-  clean_grub_dir (dst_locale);
+
+  grub_install_copy_nls(src, dst);
 
   if (install_modules.is_default)
 copy_by_ext (src, dst_platform, ".mod", 1);
@@ -797,50 +852,6 @@ grub_install_copy_files (const char *src,
   free (dstf);
 }
 
-  if (install_locales.is_default)
-{
-  char *srcd = grub_util_path_concat (2, src, "po");
-  copy_by_ext (srcd, dst_locale, ".mo", 0);
-  copy_locales (dst_locale);
-  free (srcd);
-}
-  else
-{
-  const char *locale_dir = get_localedir ();
-
-  for (i = 0; i < install_locales.n_entries; i++)
-   {
- char *srcf = grub_util_path_concat_ext (3, src,
-   "po",
-   install_locales.entries[i],
-   ".mo");
- char *dstf = grub_util_path_concat_ext (2, dst_locale,
-   install_locales.entries[i],
-   ".mo");
- if (grub_install_compress_file (srcf, dstf, 0))
-   {
- free (srcf);
- free (dstf);
- continue;
-   }
- free (srcf);
- srcf = grub_util_path_concat_ext (4,
-locale_dir,
-install_locales.entries[i],
-