[PATCH 1/3] Make UTF-8 output simpler and easier to read.
Michael Matz
matz@suse.de
Wed Oct 8 12:48:03 GMT 2025
Hello,
On Wed, 8 Oct 2025, Andrew C Aitchison wrote:
> >> It is all subjective preference, but the project doesn't seem to
> >> make its preferences clear for me to follow.
> >
> > It has such style (and it's more than a preference), but it's indeed true
> > that they are meanwhile only learnable (if that's a word) by submitting
> > patches and becoming corrected, or using emacs gnu default or indent :)
>
> Hmm. I don't see .indent.pro anywhere.
Wouldn't have helped. See below.
> # for i in nm.c readelf.c strings.c objdump.c ; do echo -n "$i " ;
> indent -gnu $i -o $i.gnu ; diff -w $i $i.gnu | wc -l ; done
> nm.c 220
> readelf.c 6084
> strings.c 78
> objdump.c 573
>
> That is, ignoring whitespace but including line-breaks, hundreds
> or thousands of differences between what indent thinks is gnu style
> and what we currently have in each file (these are the files I'm
> currently trying to patch).
Yes, but the above command (or all the options that '-gnu' expands to)
does indeed format stuff the way its expected when religiously following
the rules. For instance:
- case PT_RISCV_ATTRIBUTES: return "RISCV_ATTRIBUTES";
- default: return NULL;
+ case PT_RISCV_ATTRIBUTES:
+ return "RISCV_ATTRIBUTES";
+ default:
+ return NULL;
The latter is normal GNU style. _But_ readelf.c uses the former style for
many of its case statements, of which it has very many all doing similar
things. The standard documents aren't only hard rules, it allows other
things when it makes sense, and here it does. As soon as that is the case
a tool will always have a hard time as it tends to apply rules meticously,
even when a human wouldn't. Another example:
char * strtab,
uint64_t strtablen,
- relocation_type rel_type,
- bool is_dynsym)
+ relocation_type rel_type, bool is_dynsym)
{
Part of a function def. Normally function args are just left aligned
lists, which is what indent did (but only for the last args, which may
even be a bug). But the author decided it would be nicer (as there are
many args) in this specific case to align them different and have one per
line. Well, another one-off preference a tool can't readily handle. As
is usual with relaxed application of rules, or in situations where
multiple acceptable alternatives exist, adhere to pre-existing practice,
i.e. here: the style of the existing surrounding code.
There are more types of changes indent does, which are either "correct":
- print_relr_addr_and_sym (filedata, symtab, nsyms, strtab, addr);
+ print_relr_addr_and_sym (filedata, symtab, nsyms, strtab,
+ addr);
(breaking a long line), or could be considered a wart in indent:
- warn (_("Unusual RELR bitmap - no previous entry to set the base address\n"));
+ warn (_
+ ("Unusual RELR bitmap - no previous entry to set the base address\n"));
(also breaking a long line, but not achieving the goal anyway, so the
change was just useless), or an borderline bug in indent:
- else if (valid_dynamic_name (filedata, filedata->dynamic_symbols[i].st_name))
- ...
- ...
+ else
+ if (valid_dynamic_name
+ (filedata, filedata->dynamic_symbols[i].st_name))
(the necessary linebreak should have been after 'filedata,', not
separating else and if).
Having said all that, the output of indent on these couple files would
indeed be considered "correct". That doesn't mean that the text before
these changes is "incorrect".
> If the corner cases are that important I would think it worth the effort
> to get them fixed in one of these tools so that the style can be
> auto-applied in the submission/check-in process.
Probably. I guess its a matter of "patches welcome".
Ciao,
Michael.
More information about the Binutils
mailing list