[PATCH 0/1] PRI* translation issues.

[PATCH 0/1] PRI* translation issues.

[Miguel Ángel Arruga Vivas]

Hi,

Daniel Kiper <dkiper@net-space.pl> writes:
[...]
> Thank you for the report!

You're welcome! Sorry for the delay again, I've been a bit busy.

[...]
> Could you send this patch using "git send-email"?

I hope the updated patch works fine.

I have some doubts regarding this acryonym:

> Could you add your SOB here?

I added a description of the change there too instead of only making
the reference to the manual, but certainly I don't know what SOB means
in this context, sorry. :-(

Regarding the patch:

>> +		      char rel_info[17]; /* log16 (2^64) = 16, plus nul.  */
>
> Please add empty line here and s/nul/NUL/.

I've added the empty line and modified the NUL as requested.

>> +		      return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
>> +					 N_("relocation 0x%s"
>> +					    " is not implemented yet"),
>> +					 rel_info);
>
> Please merge all these 3 lines into one, i.e:
>   N_("relocation 0x%s" is not implemented yet"), rel_info);

I wasn't quite sure about this, as the column count goes high on my
Emacs, should I remove some indentation?  Perhaps three lines could be
another compromise with N_ and rel_info on separate lines as the other
code?

> Does this patch fix all PRI* translation issues?

I think all PRI* issues facing up the user are covered; I grep'ed the
code and they seem to be only used for internal messages.

> When I get next version of patch from you I will rebuild and update pot file.

Sorry again for the delay, and thank you very much for your support.

Happy hacking!
Miguel


Miguel Ángel Arruga Vivas (1):
  i18n: Format large integers before the translation message.

 grub-core/disk/luks2.c             |  5 ++++-
 grub-core/efiemu/i386/loadcore64.c | 13 +++++++++----
 grub-core/kern/arm64/dl.c          | 13 +++++++++----
 grub-core/kern/ia64/dl.c           | 13 +++++++++----
 grub-core/kern/riscv/dl.c          | 14 ++++++++++----
 grub-core/kern/sparc64/dl.c        | 12 ++++++++----
 grub-core/kern/x86_64/dl.c         | 13 +++++++++----
 7 files changed, 58 insertions(+), 25 deletions(-)

-- 
2.30.0

[PATCH 1/1] i18n: Format large integers before the translation message.

[Miguel Ángel Arruga Vivas]

GNU gettext only supports C99 macros for integral types, more specific
macros should be used to format the number to a string before the
internationalization, as explained on the section of gettext's manual
"Preparing Strings":
<http://www.gnu.org/software/gettext/manual/html_node/Preparing-Strings.html#No-string-concatenation>

The function grub_snprintf is used to print the numeric values to a
intermediate string buffer and the internationalized message contains
a string format directive instead.
---
 grub-core/disk/luks2.c             |  5 ++++-
 grub-core/efiemu/i386/loadcore64.c | 13 +++++++++----
 grub-core/kern/arm64/dl.c          | 13 +++++++++----
 grub-core/kern/ia64/dl.c           | 13 +++++++++----
 grub-core/kern/riscv/dl.c          | 14 ++++++++++----
 grub-core/kern/sparc64/dl.c        | 12 ++++++++----
 grub-core/kern/x86_64/dl.c         | 13 +++++++++----
 7 files changed, 58 insertions(+), 25 deletions(-)

diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
index 125e8609a..5bedbd6f9 100644
--- a/grub-core/disk/luks2.c
+++ b/grub-core/disk/luks2.c
@@ -612,6 +612,7 @@ luks2_recover_key (grub_disk_t source,
   /* Try all keyslot */
   for (json_idx = 0; json_idx < size; json_idx++)
     {
+      char indexstr[21]; /* log10 (2^64) ~ 20, plus NUL character.  */
       typeof (source->total_sectors) max_crypt_sectors = 0;
 
       grub_errno = GRUB_ERR_NONE;
@@ -732,11 +733,13 @@ luks2_recover_key (grub_disk_t source,
 	  continue;
 	}
 
+      grub_snprintf (indexstr, sizeof (indexstr) - 1, "%" PRIuGRUB_UINT64_T,
+		     keyslot.idx);
       /*
        * TRANSLATORS: It's a cryptographic key slot: one element of an array
        * where each element is either empty or holds a key.
        */
-      grub_printf_ (N_("Slot \"%" PRIuGRUB_UINT64_T "\" opened\n"), keyslot.idx);
+      grub_printf_ (N_("Slot \"%s\" opened\n"), indexstr);
 
       candidate_key_len = keyslot.key_size;
       break;
diff --git a/grub-core/efiemu/i386/loadcore64.c b/grub-core/efiemu/i386/loadcore64.c
index 7316efc39..2d1d8245a 100644
--- a/grub-core/efiemu/i386/loadcore64.c
+++ b/grub-core/efiemu/i386/loadcore64.c
@@ -121,10 +121,15 @@ grub_arch_efiemu_relocate_symbols64 (grub_efiemu_segment_t segs,
 		      return err;
                     break;
 		  default:
-		    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-				       N_("relocation 0x%" PRIxGRUB_UINT64_T
-					  " is not implemented yet"),
-				       ELF_R_TYPE (rel->r_info));
+		    {
+		      char rel_info[17]; /* log16 (2^64) = 16, plus NUL.  */
+
+		      grub_snprintf (rel_info, sizeof (rel_info) - 1,
+				     "%" PRIxGRUB_UINT64_T,
+				     ELF_R_TYPE (rel->r_info));
+		      return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+					 N_("relocation 0x%s is not implemented yet"), rel_info);
+		    }
 		  }
 	      }
 	  }
diff --git a/grub-core/kern/arm64/dl.c b/grub-core/kern/arm64/dl.c
index 401672374..f612871b9 100644
--- a/grub-core/kern/arm64/dl.c
+++ b/grub-core/kern/arm64/dl.c
@@ -183,10 +183,15 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
 	  break;
 
 	default:
-	  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			     N_("relocation 0x%" PRIxGRUB_UINT64_T
-				" is not implemented yet"),
-			     ELF_R_TYPE (rel->r_info));
+	  {
+	    char rel_info[17]; /* log16 (2^64) = 16, plus NUL.  */
+
+	    grub_snprintf (rel_info, sizeof (rel_info) - 1,
+			   "%" PRIxGRUB_UINT64_T, ELF_R_TYPE (rel->r_info));
+	    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+			       N_("relocation 0x%s is not implemented yet"),
+			       rel_info);
+	  }
 	}
     }
 
diff --git a/grub-core/kern/ia64/dl.c b/grub-core/kern/ia64/dl.c
index b19706c50..ec65d5890 100644
--- a/grub-core/kern/ia64/dl.c
+++ b/grub-core/kern/ia64/dl.c
@@ -136,10 +136,15 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
 	case R_IA64_LDXMOV:
 	  break;
 	default:
-	  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			     N_("relocation 0x%" PRIxGRUB_UINT64_T
-				" is not implemented yet"),
-			     ELF_R_TYPE (rel->r_info));
+	  {
+	    char rel_info[17]; /* log16 (2^64) = 16, plus NUL.  */
+
+	    grub_snprintf (rel_info, sizeof (rel_info) - 1,
+			   "%" PRIxGRUB_UINT64_T, ELF_R_TYPE (rel->r_info));
+	    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+			       N_("relocation 0x%s is not implemented yet"),
+			       rel_info);
+	  }
 	}
     }
   return GRUB_ERR_NONE;
diff --git a/grub-core/kern/riscv/dl.c b/grub-core/kern/riscv/dl.c
index d78297eee..53a00fb46 100644
--- a/grub-core/kern/riscv/dl.c
+++ b/grub-core/kern/riscv/dl.c
@@ -330,10 +330,16 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
 	case R_RISCV_RELAX:
 	  break;
 	default:
-	  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			     N_("relocation 0x%" PRIxGRUB_UINT64_T
-				" is not implemented yet"),
-			     (grub_uint64_t) ELF_R_TYPE (rel->r_info));
+	  {
+	    char rel_info[17]; /* log16 (2^64) = 16, plus NUL.  */
+
+	    grub_snprintf (rel_info, sizeof (rel_info) - 1,
+			   "%" PRIxGRUB_UINT64_T,
+			   (grub_uint64_t) ELF_R_TYPE (rel->r_info));
+	    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+			       N_("relocation 0x%s is not implemented yet"),
+			       rel_info);
+	  }
 	}
     }
 
diff --git a/grub-core/kern/sparc64/dl.c b/grub-core/kern/sparc64/dl.c
index bbcce8ed5..975fca88d 100644
--- a/grub-core/kern/sparc64/dl.c
+++ b/grub-core/kern/sparc64/dl.c
@@ -176,10 +176,14 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
 	       & 0x1fff);
 	  break;
 	default:
-	  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			     N_("relocation 0x%" PRIxGRUB_UINT64_T
-				" is not implemented yet"),
-			     ELF_R_TYPE (rel->r_info));
+	  {
+	    char rel_info[17]; /* log16 (2^64) = 16, plus NUL.  */
+
+	    grub_snprintf (rel_info, sizeof (rel_info) - 1,
+			   "%" PRIxGRUB_UINT64_T, ELF_R_TYPE (rel->r_info));
+	    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+			       N_("relocation 0x%s is not implemented yet"),
+			       rel_info);
 	}
     }
 
diff --git a/grub-core/kern/x86_64/dl.c b/grub-core/kern/x86_64/dl.c
index 1af5a0eeb..c371aa8de 100644
--- a/grub-core/kern/x86_64/dl.c
+++ b/grub-core/kern/x86_64/dl.c
@@ -106,10 +106,15 @@ grub_arch_dl_relocate_symbols (grub_dl_t mod, void *ehdr,
 	  break;
 
 	default:
-	  return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
-			     N_("relocation 0x%" PRIxGRUB_UINT64_T
-			        " is not implemented yet"),
-			     ELF_R_TYPE (rel->r_info));
+	  {
+	    char rel_info[17]; /* log16 (2^64) = 16, plus NUL.  */
+
+	    grub_snprintf (rel_info, sizeof (rel_info) - 1,
+			   "%" PRIxGRUB_UINT64_T, ELF_R_TYPE (rel->r_info));
+	    return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+			       N_("relocation 0x%s is not implemented yet"),
+			       rel_info);
+	  }
 	}
     }
 
-- 
2.30.0

Re: [PATCH 0/1] PRI* translation issues.

[Mihai Moldovan]

[-- Attachment #1.1: Type: text/plain, Size: 442 bytes --]

* On 4/3/21 3:33 PM, Miguel Ángel Arruga Vivas wrote:
> Daniel Kiper <dkiper@net-space.pl> writes:
> I have some doubts regarding this acryonym:
> 
>> Could you add your SOB here?
> 
> I added a description of the change there too instead of only making
> the reference to the manual, but certainly I don't know what SOB means
> in this context, sorry. :-(

SOB means Signed-Off-By. See the -s flag to git-commit(1).


Mihai


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 0/1] PRI* translation issues.

[Daniel Kiper]

Hey,

On Sat, Apr 03, 2021 at 03:33:32PM +0200, Miguel Ángel Arruga Vivas wrote:
> Hi,
>
> Daniel Kiper <dkiper@net-space.pl> writes:
> [...]
> > Thank you for the report!
>
> You're welcome! Sorry for the delay again, I've been a bit busy.

No problem.

> [...]
> > Could you send this patch using "git send-email"?
>
> I hope the updated patch works fine.

LGTM, so, Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>

> I have some doubts regarding this acryonym:
>
> > Could you add your SOB here?

Signed-off-by: Miguel Ángel Arruga Vivas <rosen644835@gmail.com>

I can add this for you before committing the patch.

> I added a description of the change there too instead of only making
> the reference to the manual, but certainly I don't know what SOB means

Great! Though I will tweak it a bit before committing.

> in this context, sorry. :-(

No worries.

> Regarding the patch:
>
> >> +		      char rel_info[17]; /* log16 (2^64) = 16, plus nul.  */
> >
> > Please add empty line here and s/nul/NUL/.
>
> I've added the empty line and modified the NUL as requested.
>
> >> +		      return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
> >> +					 N_("relocation 0x%s"
> >> +					    " is not implemented yet"),
> >> +					 rel_info);
> >
> > Please merge all these 3 lines into one, i.e:
> >   N_("relocation 0x%s" is not implemented yet"), rel_info);
>
> I wasn't quite sure about this, as the column count goes high on my
> Emacs, should I remove some indentation?  Perhaps three lines could be
> another compromise with N_ and rel_info on separate lines as the other
> code?

Yep.

> > Does this patch fix all PRI* translation issues?
>
> I think all PRI* issues facing up the user are covered; I grep'ed the
> code and they seem to be only used for internal messages.

Perfect!

> > When I get next version of patch from you I will rebuild and update pot file.
>
> Sorry again for the delay, and thank you very much for your support.

You are welcome!

Daniel

Re: [PATCH 0/1] PRI* translation issues.

[Miguel Ángel Arruga Vivas]

Hi,

Daniel Kiper <dkiper@net-space.pl> writes:

[...]
> Signed-off-by: Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
>
> I can add this for you before committing the patch.

That'd be great, thank you.

>> I added a description of the change there too instead of only making
>> the reference to the manual, but certainly I don't know what SOB means
>
> Great! Though I will tweak it a bit before committing.

Thanks again. I'm not a great writer even on my mother tongue, any
improvement is always welcome.

[...]
>> I wasn't quite sure about this, as the column count goes high on my
>> Emacs, should I remove some indentation?  Perhaps three lines could be
>> another compromise with N_ and rel_info on separate lines as the other
>> code?
>
> Yep.

Which one of the three options do you mean? In any case, it goes without
saying that I'm thankful to any tweak performed before pushing the patch
to the working branch.

Best regards,
Miguel

Re: [PATCH 0/1] PRI* translation issues.

[Miguel Ángel Arruga Vivas]

Hi,

Mihai Moldovan <ionic@ionic.de> writes:

> * On 4/3/21 3:33 PM, Miguel Ángel Arruga Vivas wrote:
>> Daniel Kiper <dkiper@net-space.pl> writes:
>> I have some doubts regarding this acryonym:
>> 
>>> Could you add your SOB here?
>> 
>> I added a description of the change there too instead of only making
>> the reference to the manual, but certainly I don't know what SOB means
>> in this context, sorry. :-(
>
> SOB means Signed-Off-By. See the -s flag to git-commit(1).

Thank you very much for the explanation; it makes total sense now.

Best regards,
Miguel

Re: [PATCH 0/1] PRI* translation issues.

[Daniel Kiper]

Hey Miguel,

On Thu, Apr 15, 2021 at 10:58:38AM +0200, Miguel Ángel Arruga Vivas wrote:
> Hi,
>
> Daniel Kiper <dkiper@net-space.pl> writes:
>
> [...]
> > Signed-off-by: Miguel Ángel Arruga Vivas <rosen644835@gmail.com>
> >
> > I can add this for you before committing the patch.
>
> That'd be great, thank you.
>
> >> I added a description of the change there too instead of only making
> >> the reference to the manual, but certainly I don't know what SOB means
> >
> > Great! Though I will tweak it a bit before committing.
>
> Thanks again. I'm not a great writer even on my mother tongue, any
> improvement is always welcome.
>
> [...]
> >> I wasn't quite sure about this, as the column count goes high on my
> >> Emacs, should I remove some indentation?  Perhaps three lines could be
> >> another compromise with N_ and rel_info on separate lines as the other
> >> code?
> >
> > Yep.
>
> Which one of the three options do you mean? In any case, it goes without
> saying that I'm thankful to any tweak performed before pushing the patch
> to the working branch.

Here [1] is your updated patch. I have checked the translation file once
again and spotted one more issue. I fixed it in this [2] patch. I will merge
it when I get a review. Then I will rebuild and repost translation file.

Daniel

[1] http://git.savannah.gnu.org/gitweb/?p=grub.git;a=commit;h=837fe48deb89b31710904de4e9c7dda258490e1c
[2] https://lists.gnu.org/archive/html/grub-devel/2021-04/msg00049.html