[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],
- 

Re: Multiboot ELF segment handling patch

2018-04-13 Thread Alexander Boettcher
Hello,

On 06.04.2018 14:28, Daniel Kiper wrote:
> On Thu, Mar 29, 2018 at 11:20:37AM +0200, Alexander Boettcher wrote:
>
>> Can you please have a look and check regarding what should/could be
>> changed to get it upstream? We did not test the dynamic relocation part,
> 
> Sure thing, please take a look below...
> 
>> since we have no such (kernel) setup. Thanks in advance.
> 
> You can use Xen (git://xenbits.xen.org/xen.git) for tests.
> Just compile hypervisor without any tools and use xen.gz.
> It produces nice output and you can see it is relocated or not.
> Of course you have to use Multiboot2 protocol.

Thanks, I managed to setup it. Based on your comments and on the Xen
test case I had to re-work the patch, so that it now works for
relocation and non-relocation kernels.

Please review again.
> However, this does not mean that I will not take this patch. Though
> it requires some tweaking.
> 
> First of all, lack of SOB.

What is a SOB ?

Cheers,

-- 
Alexander Boettcher
Genode Labs

http://www.genode-labs.com - http://www.genode.org

Genode Labs GmbH - Amtsgericht Dresden - HRB 28424 - Sitz Dresden
Geschäftsführer: Dr.-Ing. Norman Feske, Christian Helmuth
From c37727840be8aeb81ea4bbc95ac09a1b1e3d3658 Mon Sep 17 00:00:00 2001
From: Alexander Boettcher 
Date: Fri, 13 Apr 2018 23:37:01 +0200
Subject: [PATCH] mbi: use per segment a separate relocator chunk

if elf is non relocatable.

If the segments are not dense packed, the original code set up a huge
relocator chunk comprising all segments.

During the final relocation in grub_relocator_prepare_relocs, the chunk
is moved to its desired place and overrides memory which are actually not
covered/touched by the specified segments.

The overriden memory may contain device memory (vga text mode e.g.), which
leads to strange boot behaviour.
---
 grub-core/loader/multiboot_elfxx.c | 56 --
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/grub-core/loader/multiboot_elfxx.c b/grub-core/loader/multiboot_elfxx.c
index 67daf59..d936223 100644
--- a/grub-core/loader/multiboot_elfxx.c
+++ b/grub-core/loader/multiboot_elfxx.c
@@ -57,9 +57,9 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
   char *phdr_base;
   grub_err_t err;
   grub_relocator_chunk_t ch;
-  grub_uint32_t load_offset, load_size;
+  grub_uint32_t load_offset = 0, load_size;
   int i;
-  void *source;
+  void *source = NULL;
 
   if (ehdr->e_ident[EI_MAG0] != ELFMAG0
   || ehdr->e_ident[EI_MAG1] != ELFMAG1
@@ -83,6 +83,7 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
 #define phdr(i)			((Elf_Phdr *) (phdr_base + (i) * ehdr->e_phentsize))
 
   mld->link_base_addr = ~0;
+  mld->load_base_addr = ~0;
 
   /* Calculate lowest and highest load address.  */
   for (i = 0; i < ehdr->e_phnum; i++)
@@ -108,27 +109,19 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
 	  mld->min_addr, mld->max_addr - load_size,
 	  load_size, mld->align ? mld->align : 1,
 	  mld->preference, mld->avoid_efi_boot_services);
-}
-  else
-err = grub_relocator_alloc_chunk_addr (GRUB_MULTIBOOT (relocator), &ch,
-	   mld->link_base_addr, load_size);
 
-  if (err)
-{
-  grub_dprintf ("multiboot_loader", "Cannot allocate memory for OS image\n");
-  return err;
-}
+  if (err)
+{
+  grub_dprintf ("multiboot_loader", "Cannot allocate memory for OS image\n");
+  return err;
+}
 
-  mld->load_base_addr = get_physical_target_address (ch);
-  source = get_virtual_current_address (ch);
+  mld->load_base_addr = get_physical_target_address (ch);
+  source = get_virtual_current_address (ch);
 
-  grub_dprintf ("multiboot_loader", "link_base_addr=0x%x, load_base_addr=0x%x, "
-		"load_size=0x%x, relocatable=%d\n", mld->link_base_addr,
-		mld->load_base_addr, load_size, mld->relocatable);
-
-  if (mld->relocatable)
-grub_dprintf ("multiboot_loader", "align=0x%lx, preference=0x%x, avoid_efi_boot_services=%d\n",
-		  (long) mld->align, mld->preference, mld->avoid_efi_boot_services);
+  grub_dprintf ("multiboot_loader", "align=0x%lx, preference=0x%x, avoid_efi_boot_services=%d\n",
+		 (long) mld->align, mld->preference, mld->avoid_efi_boot_services);
+}
 
   /* Load every loadable segment in memory.  */
   for (i = 0; i < ehdr->e_phnum; i++)
@@ -139,7 +132,24 @@ CONCAT(grub_multiboot_load_elf, XX) (mbi_load_data_t *mld)
 	  grub_dprintf ("multiboot_loader", "segment %d: paddr=0x%lx, memsz=0x%lx, vaddr=0x%lx\n",
 			i, (long) phdr(i)->p_paddr, (long) phdr(i)->p_memsz, (long) phdr(i)->p_vaddr);
 
-	  load_offset = phdr(i)->p_paddr - mld->link_base_addr;
+	  if (mld->relocatable)
+	load_offset = phdr(i)->p_paddr - mld->link_base_addr;
+	  else
+	{
+	  err = grub_relocator_alloc_chunk_addr (GRUB_MULTIBOOT (relocator), &ch,
+	 phdr(i)->p_paddr, phdr(i)->p_memsz);
+
+	  if (err)
+		{
+		  grub_dpr