[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