* [PATCH] x86: drop print_operand_value()'s "hex" parameter
@ 2022-06-14 10:06 Jan Beulich
2022-06-14 13:59 ` H.J. Lu
2022-06-15 15:07 ` Michael Matz
0 siblings, 2 replies; 6+ messages in thread
From: Jan Beulich @ 2022-06-14 10:06 UTC (permalink / raw)
To: Binutils
For quite some time all callers have been passing 1 / true. While there
fold the final oappend_with_style() calls.
---
Instead of 0x%x I could see us using %#x, thus printing at least zero
without the redundant 0x prefix. Thoughts? (I expect there'll be quite a
bit of testsuite fallout from such a change, but I don't think that's a
major concern.)
--- a/opcodes/i386-dis.c
+++ b/opcodes/i386-dis.c
@@ -10969,61 +10969,22 @@ OP_indirE (instr_info *ins, int bytemode
}
static void
-print_operand_value (instr_info *ins, bool hex, bfd_vma disp,
+print_operand_value (instr_info *ins, bfd_vma disp,
enum disassembler_style style)
{
char tmp[30];
+ unsigned int i = 0;
if (ins->address_mode == mode_64bit)
{
- if (hex)
- {
- int i;
- oappend_with_style (ins, "0x", style);
- sprintf_vma (tmp, disp);
- for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
- oappend_with_style (ins, tmp + i, style);
- }
- else
- {
- bfd_signed_vma v = disp;
- int i;
- if (v < 0)
- {
- oappend_char_with_style (ins, '-', style);
- v = -disp;
- /* Check for possible overflow on 0x8000000000000000. */
- if (v < 0)
- {
- oappend_with_style (ins, "9223372036854775808", style);
- return;
- }
- }
- if (!v)
- {
- oappend_char_with_style (ins, '0', style);
- return;
- }
-
- i = 0;
- tmp[29] = 0;
- while (v)
- {
- tmp[28 - i] = (v % 10) + '0';
- v /= 10;
- i++;
- }
- oappend_with_style (ins, tmp + 29 - i, style);
- }
+ oappend_with_style (ins, "0x", style);
+ sprintf_vma (tmp, disp);
+ while (tmp[i] == '0' && tmp[i + 1])
+ ++i;
}
else
- {
- if (hex)
- sprintf (tmp, "0x%x", (unsigned int) disp);
- else
- sprintf (tmp, "%d", (int) disp);
- oappend_with_style (ins, tmp, style);
- }
+ sprintf (tmp, "0x%x", (unsigned int) disp);
+ oappend_with_style (ins, tmp + i, style);
}
/* Like oappend, but called for immediate operands. */
@@ -11033,7 +10994,7 @@ oappend_immediate (instr_info *ins, bfd_
{
if (!ins->intel_syntax)
oappend_char_with_style (ins, '$', dis_style_immediate);
- print_operand_value (ins, true, imm, dis_style_immediate);
+ print_operand_value (ins, imm, dis_style_immediate);
}
/* Put DISP in BUF as signed hex number. */
@@ -11721,7 +11682,7 @@ OP_E_memory (instr_info *ins, int bytemo
if (havedisp || riprel)
print_displacement (ins, disp);
else
- print_operand_value (ins, true, disp, dis_style_address_offset);
+ print_operand_value (ins, disp, dis_style_address_offset);
if (riprel)
{
set_op (ins, disp, true);
@@ -11798,7 +11759,7 @@ OP_E_memory (instr_info *ins, int bytemo
if (havedisp)
print_displacement (ins, disp);
else
- print_operand_value (ins, true, disp, dis_style_address_offset);
+ print_operand_value (ins, disp, dis_style_address_offset);
}
oappend_char (ins, ins->close_char);
@@ -11825,7 +11786,7 @@ OP_E_memory (instr_info *ins, int bytemo
oappend_register (ins, att_names_seg[ds_reg - es_reg]);
oappend (ins, ":");
}
- print_operand_value (ins, true, disp, dis_style_text);
+ print_operand_value (ins, disp, dis_style_text);
}
}
}
@@ -11900,7 +11861,7 @@ OP_E_memory (instr_info *ins, int bytemo
oappend_register (ins, att_names_seg[ds_reg - es_reg]);
oappend (ins, ":");
}
- print_operand_value (ins, true, disp & 0xffff, dis_style_text);
+ print_operand_value (ins, disp & 0xffff, dis_style_text);
}
}
if (ins->vex.b)
@@ -12370,7 +12331,7 @@ OP_J (instr_info *ins, int bytemode, int
disp = ((ins->start_pc + (ins->codep - ins->start_codep) + disp) & mask)
| segment;
set_op (ins, disp, false);
- print_operand_value (ins, true, disp, dis_style_text);
+ print_operand_value (ins, disp, dis_style_text);
}
static void
@@ -12430,7 +12391,7 @@ OP_OFF (instr_info *ins, int bytemode, i
oappend (ins, ":");
}
}
- print_operand_value (ins, true, off, dis_style_address_offset);
+ print_operand_value (ins, off, dis_style_address_offset);
}
static void
@@ -12459,7 +12420,7 @@ OP_OFF64 (instr_info *ins, int bytemode,
oappend (ins, ":");
}
}
- print_operand_value (ins, true, off, dis_style_address_offset);
+ print_operand_value (ins, off, dis_style_address_offset);
}
static void
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] x86: drop print_operand_value()'s "hex" parameter
2022-06-14 10:06 [PATCH] x86: drop print_operand_value()'s "hex" parameter Jan Beulich
@ 2022-06-14 13:59 ` H.J. Lu
2022-06-14 14:54 ` Jan Beulich
2022-06-15 15:07 ` Michael Matz
1 sibling, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2022-06-14 13:59 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
On Tue, Jun 14, 2022 at 3:07 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> For quite some time all callers have been passing 1 / true. While there
> fold the final oappend_with_style() calls.
> ---
> Instead of 0x%x I could see us using %#x, thus printing at least zero
> without the redundant 0x prefix. Thoughts? (I expect there'll be quite a
0x is clearer than 0 which is also used for octal.
> bit of testsuite fallout from such a change, but I don't think that's a
> major concern.)
>
> --- a/opcodes/i386-dis.c
> +++ b/opcodes/i386-dis.c
> @@ -10969,61 +10969,22 @@ OP_indirE (instr_info *ins, int bytemode
> }
>
> static void
> -print_operand_value (instr_info *ins, bool hex, bfd_vma disp,
> +print_operand_value (instr_info *ins, bfd_vma disp,
> enum disassembler_style style)
> {
> char tmp[30];
> + unsigned int i = 0;
>
> if (ins->address_mode == mode_64bit)
> {
> - if (hex)
> - {
> - int i;
> - oappend_with_style (ins, "0x", style);
> - sprintf_vma (tmp, disp);
> - for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
> - oappend_with_style (ins, tmp + i, style);
> - }
> - else
> - {
> - bfd_signed_vma v = disp;
> - int i;
> - if (v < 0)
> - {
> - oappend_char_with_style (ins, '-', style);
> - v = -disp;
> - /* Check for possible overflow on 0x8000000000000000. */
> - if (v < 0)
> - {
> - oappend_with_style (ins, "9223372036854775808", style);
> - return;
> - }
> - }
> - if (!v)
> - {
> - oappend_char_with_style (ins, '0', style);
> - return;
> - }
> -
> - i = 0;
> - tmp[29] = 0;
> - while (v)
> - {
> - tmp[28 - i] = (v % 10) + '0';
> - v /= 10;
> - i++;
> - }
> - oappend_with_style (ins, tmp + 29 - i, style);
> - }
> + oappend_with_style (ins, "0x", style);
> + sprintf_vma (tmp, disp);
> + while (tmp[i] == '0' && tmp[i + 1])
> + ++i;
> }
> else
> - {
> - if (hex)
> - sprintf (tmp, "0x%x", (unsigned int) disp);
> - else
> - sprintf (tmp, "%d", (int) disp);
> - oappend_with_style (ins, tmp, style);
> - }
> + sprintf (tmp, "0x%x", (unsigned int) disp);
> + oappend_with_style (ins, tmp + i, style);
> }
>
> /* Like oappend, but called for immediate operands. */
> @@ -11033,7 +10994,7 @@ oappend_immediate (instr_info *ins, bfd_
> {
> if (!ins->intel_syntax)
> oappend_char_with_style (ins, '$', dis_style_immediate);
> - print_operand_value (ins, true, imm, dis_style_immediate);
> + print_operand_value (ins, imm, dis_style_immediate);
> }
>
> /* Put DISP in BUF as signed hex number. */
> @@ -11721,7 +11682,7 @@ OP_E_memory (instr_info *ins, int bytemo
> if (havedisp || riprel)
> print_displacement (ins, disp);
> else
> - print_operand_value (ins, true, disp, dis_style_address_offset);
> + print_operand_value (ins, disp, dis_style_address_offset);
> if (riprel)
> {
> set_op (ins, disp, true);
> @@ -11798,7 +11759,7 @@ OP_E_memory (instr_info *ins, int bytemo
> if (havedisp)
> print_displacement (ins, disp);
> else
> - print_operand_value (ins, true, disp, dis_style_address_offset);
> + print_operand_value (ins, disp, dis_style_address_offset);
> }
>
> oappend_char (ins, ins->close_char);
> @@ -11825,7 +11786,7 @@ OP_E_memory (instr_info *ins, int bytemo
> oappend_register (ins, att_names_seg[ds_reg - es_reg]);
> oappend (ins, ":");
> }
> - print_operand_value (ins, true, disp, dis_style_text);
> + print_operand_value (ins, disp, dis_style_text);
> }
> }
> }
> @@ -11900,7 +11861,7 @@ OP_E_memory (instr_info *ins, int bytemo
> oappend_register (ins, att_names_seg[ds_reg - es_reg]);
> oappend (ins, ":");
> }
> - print_operand_value (ins, true, disp & 0xffff, dis_style_text);
> + print_operand_value (ins, disp & 0xffff, dis_style_text);
> }
> }
> if (ins->vex.b)
> @@ -12370,7 +12331,7 @@ OP_J (instr_info *ins, int bytemode, int
> disp = ((ins->start_pc + (ins->codep - ins->start_codep) + disp) & mask)
> | segment;
> set_op (ins, disp, false);
> - print_operand_value (ins, true, disp, dis_style_text);
> + print_operand_value (ins, disp, dis_style_text);
> }
>
> static void
> @@ -12430,7 +12391,7 @@ OP_OFF (instr_info *ins, int bytemode, i
> oappend (ins, ":");
> }
> }
> - print_operand_value (ins, true, off, dis_style_address_offset);
> + print_operand_value (ins, off, dis_style_address_offset);
> }
>
> static void
> @@ -12459,7 +12420,7 @@ OP_OFF64 (instr_info *ins, int bytemode,
> oappend (ins, ":");
> }
> }
> - print_operand_value (ins, true, off, dis_style_address_offset);
> + print_operand_value (ins, off, dis_style_address_offset);
> }
>
> static void
--
H.J.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] x86: drop print_operand_value()'s "hex" parameter
2022-06-14 13:59 ` H.J. Lu
@ 2022-06-14 14:54 ` Jan Beulich
2022-06-14 15:05 ` H.J. Lu
0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2022-06-14 14:54 UTC (permalink / raw)
To: H.J. Lu; +Cc: Binutils
On 14.06.2022 15:59, H.J. Lu wrote:
> On Tue, Jun 14, 2022 at 3:07 AM Jan Beulich <jbeulich@suse.com> wrote:
>>
>> For quite some time all callers have been passing 1 / true. While there
>> fold the final oappend_with_style() calls.
>> ---
>> Instead of 0x%x I could see us using %#x, thus printing at least zero
>> without the redundant 0x prefix. Thoughts? (I expect there'll be quite a
>
> 0x is clearer than 0 which is also used for octal.
How is 0x0 clearer than 0? There's also no ambiguity at all with octal:
0 represents zero for all of hex, dec, and octal.
Also - any word on the patch itself? The aspect here was brought up just
to figure whether there wants to be yet another change on top.
Jan
>> bit of testsuite fallout from such a change, but I don't think that's a
>> major concern.)
>>
>> --- a/opcodes/i386-dis.c
>> +++ b/opcodes/i386-dis.c
>> @@ -10969,61 +10969,22 @@ OP_indirE (instr_info *ins, int bytemode
>> }
>>
>> static void
>> -print_operand_value (instr_info *ins, bool hex, bfd_vma disp,
>> +print_operand_value (instr_info *ins, bfd_vma disp,
>> enum disassembler_style style)
>> {
>> char tmp[30];
>> + unsigned int i = 0;
>>
>> if (ins->address_mode == mode_64bit)
>> {
>> - if (hex)
>> - {
>> - int i;
>> - oappend_with_style (ins, "0x", style);
>> - sprintf_vma (tmp, disp);
>> - for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
>> - oappend_with_style (ins, tmp + i, style);
>> - }
>> - else
>> - {
>> - bfd_signed_vma v = disp;
>> - int i;
>> - if (v < 0)
>> - {
>> - oappend_char_with_style (ins, '-', style);
>> - v = -disp;
>> - /* Check for possible overflow on 0x8000000000000000. */
>> - if (v < 0)
>> - {
>> - oappend_with_style (ins, "9223372036854775808", style);
>> - return;
>> - }
>> - }
>> - if (!v)
>> - {
>> - oappend_char_with_style (ins, '0', style);
>> - return;
>> - }
>> -
>> - i = 0;
>> - tmp[29] = 0;
>> - while (v)
>> - {
>> - tmp[28 - i] = (v % 10) + '0';
>> - v /= 10;
>> - i++;
>> - }
>> - oappend_with_style (ins, tmp + 29 - i, style);
>> - }
>> + oappend_with_style (ins, "0x", style);
>> + sprintf_vma (tmp, disp);
>> + while (tmp[i] == '0' && tmp[i + 1])
>> + ++i;
>> }
>> else
>> - {
>> - if (hex)
>> - sprintf (tmp, "0x%x", (unsigned int) disp);
>> - else
>> - sprintf (tmp, "%d", (int) disp);
>> - oappend_with_style (ins, tmp, style);
>> - }
>> + sprintf (tmp, "0x%x", (unsigned int) disp);
>> + oappend_with_style (ins, tmp + i, style);
>> }
>>
>> /* Like oappend, but called for immediate operands. */
>> @@ -11033,7 +10994,7 @@ oappend_immediate (instr_info *ins, bfd_
>> {
>> if (!ins->intel_syntax)
>> oappend_char_with_style (ins, '$', dis_style_immediate);
>> - print_operand_value (ins, true, imm, dis_style_immediate);
>> + print_operand_value (ins, imm, dis_style_immediate);
>> }
>>
>> /* Put DISP in BUF as signed hex number. */
>> @@ -11721,7 +11682,7 @@ OP_E_memory (instr_info *ins, int bytemo
>> if (havedisp || riprel)
>> print_displacement (ins, disp);
>> else
>> - print_operand_value (ins, true, disp, dis_style_address_offset);
>> + print_operand_value (ins, disp, dis_style_address_offset);
>> if (riprel)
>> {
>> set_op (ins, disp, true);
>> @@ -11798,7 +11759,7 @@ OP_E_memory (instr_info *ins, int bytemo
>> if (havedisp)
>> print_displacement (ins, disp);
>> else
>> - print_operand_value (ins, true, disp, dis_style_address_offset);
>> + print_operand_value (ins, disp, dis_style_address_offset);
>> }
>>
>> oappend_char (ins, ins->close_char);
>> @@ -11825,7 +11786,7 @@ OP_E_memory (instr_info *ins, int bytemo
>> oappend_register (ins, att_names_seg[ds_reg - es_reg]);
>> oappend (ins, ":");
>> }
>> - print_operand_value (ins, true, disp, dis_style_text);
>> + print_operand_value (ins, disp, dis_style_text);
>> }
>> }
>> }
>> @@ -11900,7 +11861,7 @@ OP_E_memory (instr_info *ins, int bytemo
>> oappend_register (ins, att_names_seg[ds_reg - es_reg]);
>> oappend (ins, ":");
>> }
>> - print_operand_value (ins, true, disp & 0xffff, dis_style_text);
>> + print_operand_value (ins, disp & 0xffff, dis_style_text);
>> }
>> }
>> if (ins->vex.b)
>> @@ -12370,7 +12331,7 @@ OP_J (instr_info *ins, int bytemode, int
>> disp = ((ins->start_pc + (ins->codep - ins->start_codep) + disp) & mask)
>> | segment;
>> set_op (ins, disp, false);
>> - print_operand_value (ins, true, disp, dis_style_text);
>> + print_operand_value (ins, disp, dis_style_text);
>> }
>>
>> static void
>> @@ -12430,7 +12391,7 @@ OP_OFF (instr_info *ins, int bytemode, i
>> oappend (ins, ":");
>> }
>> }
>> - print_operand_value (ins, true, off, dis_style_address_offset);
>> + print_operand_value (ins, off, dis_style_address_offset);
>> }
>>
>> static void
>> @@ -12459,7 +12420,7 @@ OP_OFF64 (instr_info *ins, int bytemode,
>> oappend (ins, ":");
>> }
>> }
>> - print_operand_value (ins, true, off, dis_style_address_offset);
>> + print_operand_value (ins, off, dis_style_address_offset);
>> }
>>
>> static void
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] x86: drop print_operand_value()'s "hex" parameter
2022-06-14 14:54 ` Jan Beulich
@ 2022-06-14 15:05 ` H.J. Lu
0 siblings, 0 replies; 6+ messages in thread
From: H.J. Lu @ 2022-06-14 15:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
On Tue, Jun 14, 2022 at 7:54 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 14.06.2022 15:59, H.J. Lu wrote:
> > On Tue, Jun 14, 2022 at 3:07 AM Jan Beulich <jbeulich@suse.com> wrote:
> >>
> >> For quite some time all callers have been passing 1 / true. While there
> >> fold the final oappend_with_style() calls.
> >> ---
> >> Instead of 0x%x I could see us using %#x, thus printing at least zero
> >> without the redundant 0x prefix. Thoughts? (I expect there'll be quite a
> >
> > 0x is clearer than 0 which is also used for octal.
>
> How is 0x0 clearer than 0? There's also no ambiguity at all with octal:
> 0 represents zero for all of hex, dec, and octal.
Oh, you mean when print 0. I have no strong opinion.
> Also - any word on the patch itself? The aspect here was brought up just
> to figure whether there wants to be yet another change on top.
The patch itself is OK.
> Jan
>
> >> bit of testsuite fallout from such a change, but I don't think that's a
> >> major concern.)
> >>
> >> --- a/opcodes/i386-dis.c
> >> +++ b/opcodes/i386-dis.c
> >> @@ -10969,61 +10969,22 @@ OP_indirE (instr_info *ins, int bytemode
> >> }
> >>
> >> static void
> >> -print_operand_value (instr_info *ins, bool hex, bfd_vma disp,
> >> +print_operand_value (instr_info *ins, bfd_vma disp,
> >> enum disassembler_style style)
> >> {
> >> char tmp[30];
> >> + unsigned int i = 0;
> >>
> >> if (ins->address_mode == mode_64bit)
> >> {
> >> - if (hex)
> >> - {
> >> - int i;
> >> - oappend_with_style (ins, "0x", style);
> >> - sprintf_vma (tmp, disp);
> >> - for (i = 0; tmp[i] == '0' && tmp[i + 1]; i++);
> >> - oappend_with_style (ins, tmp + i, style);
> >> - }
> >> - else
> >> - {
> >> - bfd_signed_vma v = disp;
> >> - int i;
> >> - if (v < 0)
> >> - {
> >> - oappend_char_with_style (ins, '-', style);
> >> - v = -disp;
> >> - /* Check for possible overflow on 0x8000000000000000. */
> >> - if (v < 0)
> >> - {
> >> - oappend_with_style (ins, "9223372036854775808", style);
> >> - return;
> >> - }
> >> - }
> >> - if (!v)
> >> - {
> >> - oappend_char_with_style (ins, '0', style);
> >> - return;
> >> - }
> >> -
> >> - i = 0;
> >> - tmp[29] = 0;
> >> - while (v)
> >> - {
> >> - tmp[28 - i] = (v % 10) + '0';
> >> - v /= 10;
> >> - i++;
> >> - }
> >> - oappend_with_style (ins, tmp + 29 - i, style);
> >> - }
> >> + oappend_with_style (ins, "0x", style);
> >> + sprintf_vma (tmp, disp);
> >> + while (tmp[i] == '0' && tmp[i + 1])
> >> + ++i;
> >> }
> >> else
> >> - {
> >> - if (hex)
> >> - sprintf (tmp, "0x%x", (unsigned int) disp);
> >> - else
> >> - sprintf (tmp, "%d", (int) disp);
> >> - oappend_with_style (ins, tmp, style);
> >> - }
> >> + sprintf (tmp, "0x%x", (unsigned int) disp);
> >> + oappend_with_style (ins, tmp + i, style);
> >> }
> >>
> >> /* Like oappend, but called for immediate operands. */
> >> @@ -11033,7 +10994,7 @@ oappend_immediate (instr_info *ins, bfd_
> >> {
> >> if (!ins->intel_syntax)
> >> oappend_char_with_style (ins, '$', dis_style_immediate);
> >> - print_operand_value (ins, true, imm, dis_style_immediate);
> >> + print_operand_value (ins, imm, dis_style_immediate);
> >> }
> >>
> >> /* Put DISP in BUF as signed hex number. */
> >> @@ -11721,7 +11682,7 @@ OP_E_memory (instr_info *ins, int bytemo
> >> if (havedisp || riprel)
> >> print_displacement (ins, disp);
> >> else
> >> - print_operand_value (ins, true, disp, dis_style_address_offset);
> >> + print_operand_value (ins, disp, dis_style_address_offset);
> >> if (riprel)
> >> {
> >> set_op (ins, disp, true);
> >> @@ -11798,7 +11759,7 @@ OP_E_memory (instr_info *ins, int bytemo
> >> if (havedisp)
> >> print_displacement (ins, disp);
> >> else
> >> - print_operand_value (ins, true, disp, dis_style_address_offset);
> >> + print_operand_value (ins, disp, dis_style_address_offset);
> >> }
> >>
> >> oappend_char (ins, ins->close_char);
> >> @@ -11825,7 +11786,7 @@ OP_E_memory (instr_info *ins, int bytemo
> >> oappend_register (ins, att_names_seg[ds_reg - es_reg]);
> >> oappend (ins, ":");
> >> }
> >> - print_operand_value (ins, true, disp, dis_style_text);
> >> + print_operand_value (ins, disp, dis_style_text);
> >> }
> >> }
> >> }
> >> @@ -11900,7 +11861,7 @@ OP_E_memory (instr_info *ins, int bytemo
> >> oappend_register (ins, att_names_seg[ds_reg - es_reg]);
> >> oappend (ins, ":");
> >> }
> >> - print_operand_value (ins, true, disp & 0xffff, dis_style_text);
> >> + print_operand_value (ins, disp & 0xffff, dis_style_text);
> >> }
> >> }
> >> if (ins->vex.b)
> >> @@ -12370,7 +12331,7 @@ OP_J (instr_info *ins, int bytemode, int
> >> disp = ((ins->start_pc + (ins->codep - ins->start_codep) + disp) & mask)
> >> | segment;
> >> set_op (ins, disp, false);
> >> - print_operand_value (ins, true, disp, dis_style_text);
> >> + print_operand_value (ins, disp, dis_style_text);
> >> }
> >>
> >> static void
> >> @@ -12430,7 +12391,7 @@ OP_OFF (instr_info *ins, int bytemode, i
> >> oappend (ins, ":");
> >> }
> >> }
> >> - print_operand_value (ins, true, off, dis_style_address_offset);
> >> + print_operand_value (ins, off, dis_style_address_offset);
> >> }
> >>
> >> static void
> >> @@ -12459,7 +12420,7 @@ OP_OFF64 (instr_info *ins, int bytemode,
> >> oappend (ins, ":");
> >> }
> >> }
> >> - print_operand_value (ins, true, off, dis_style_address_offset);
> >> + print_operand_value (ins, off, dis_style_address_offset);
> >> }
> >>
> >> static void
> >
> >
> >
>
--
H.J.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: drop print_operand_value()'s "hex" parameter
2022-06-14 10:06 [PATCH] x86: drop print_operand_value()'s "hex" parameter Jan Beulich
2022-06-14 13:59 ` H.J. Lu
@ 2022-06-15 15:07 ` Michael Matz
2022-06-22 6:23 ` Jan Beulich
1 sibling, 1 reply; 6+ messages in thread
From: Michael Matz @ 2022-06-15 15:07 UTC (permalink / raw)
To: Jan Beulich; +Cc: Binutils
Hey,
On Tue, 14 Jun 2022, Jan Beulich via Binutils wrote:
> For quite some time all callers have been passing 1 / true. While there
> fold the final oappend_with_style() calls.
> ---
> Instead of 0x%x I could see us using %#x, thus printing at least zero
> without the redundant 0x prefix. Thoughts?
As you're asking for opinions, here's mine: in this case I think
consistency is better, I think there's no obvious reason to print
0 but 0xa (for instance)
and not
0x0 and 0xa
Sure, it's redundant, but IMHO that doesn't trump consistency.
Ciao,
Michael.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86: drop print_operand_value()'s "hex" parameter
2022-06-15 15:07 ` Michael Matz
@ 2022-06-22 6:23 ` Jan Beulich
0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2022-06-22 6:23 UTC (permalink / raw)
To: Michael Matz; +Cc: Binutils
On 15.06.2022 17:07, Michael Matz wrote:
> On Tue, 14 Jun 2022, Jan Beulich via Binutils wrote:
>
>> For quite some time all callers have been passing 1 / true. While there
>> fold the final oappend_with_style() calls.
>> ---
>> Instead of 0x%x I could see us using %#x, thus printing at least zero
>> without the redundant 0x prefix. Thoughts?
>
> As you're asking for opinions, here's mine: in this case I think
> consistency is better, I think there's no obvious reason to print
> 0 but 0xa (for instance)
> and not
> 0x0 and 0xa
> Sure, it's redundant, but IMHO that doesn't trump consistency.
I see. Yet my point isn't merely consistency, but also readability. To
me at least 0 is more easily recognizable as "zero" than 0x0. Indeed I
would go as far as saying the same for 1...9 as well, just that there
we'd need custom code whereas for 0 we have an easy way to omit the
leading 0x.
Jan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-22 6:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-14 10:06 [PATCH] x86: drop print_operand_value()'s "hex" parameter Jan Beulich
2022-06-14 13:59 ` H.J. Lu
2022-06-14 14:54 ` Jan Beulich
2022-06-14 15:05 ` H.J. Lu
2022-06-15 15:07 ` Michael Matz
2022-06-22 6:23 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).