tinycc-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Tinycc-devel] Source vandalism


From: Michael Matz
Subject: [Tinycc-devel] Source vandalism
Date: Wed, 29 Jul 2015 14:50:39 +0200 (CEST)
User-agent: Alpine 2.20 (LSU 67 2015-01-07)

Hello gus or Augustin,

while I appreciate more people working on tinycc, why do you think the 
best thing to do as the very first commits would be source code 
reformattings and reorganizations?  Look at what damage you've done:

@@ -204,7 +204,7 @@ void C67_g(int c)
 #endif
     ind1 = ind + 4;
     if (ind1 > (int) cur_text_section->data_allocated)
-       section_realloc(cur_text_section, ind1);
+    section_realloc(cur_text_section, ind1);

Grand, so now the conditioned statement isn't indended anymore.  Or:

     while (t) {
-       ptr = (int *) (cur_text_section->data + t);
-       {
-           Sym *sym;
+    ptr = (int *) (cur_text_section->data + t);
+    {
+        Sym *sym;

The 'ptr =' assignment has to be indended.  Or:

                 } else {
-                   qrel->r_info = ELFW(R_INFO)(0, R_X86_64_RELATIVE);
-                   qrel->r_addend = *(long long *)ptr + val;
+            qrel->r_info = ELFW(R_INFO)(0, R_X86_64_RELATIVE);
+            qrel->r_addend = *(long long *)ptr + val;
                     qrel++;
                 }

What the hell?  Parts of the conditional block are now indended completely 
wrong.  Or such changes:

-#define RC_INT     0x0001 /* generic integer register */
-#define RC_FLOAT   0x0002 /* generic float register */
-#define RC_R0      0x0004
-#define RC_R1      0x0008
-#define RC_R2      0x0010
-#define RC_R3      0x0020
-#define RC_R12     0x0040
-#define RC_F0      0x0080
-#define RC_F1      0x0100
-#define RC_F2      0x0200
-#define RC_F3      0x0400
+#define RC_INT 0x0001   /* generic integer register */
+#define RC_FLOAT 0x0002 /* generic float register */
+#define RC_R0 0x0004
+#define RC_R1 0x0008
+#define RC_R2 0x0010
+#define RC_R3 0x0020
+#define RC_R12 0x0040
+#define RC_F0 0x0080
+#define RC_F1 0x0100
+#define RC_F2 0x0200
+#define RC_F3 0x0400

Well, obviously the author of this wanted to align the numbers like in a 
table, now it looks ugly.  Or this:

 static unsigned long put_got_entry(TCCState *s1,
-                                  int reloc_type, unsigned long size, int info,
-                                  int sym_index)
+                   int reloc_type, unsigned long size, int info,
+                   int sym_index)
 {

The arguments on second and third line were meant to align with the first 
argument.  Or just a few lines down:

     if (need_plt_entry && !s1->plt) {
-       /* add PLT */
-       s1->plt = new_section(s1, ".plt", SHT_PROGBITS,
-                             SHF_ALLOC | SHF_EXECINSTR);
-       s1->plt->sh_entsize = 4;
+    /* add PLT */
+    s1->plt = new_section(s1, ".plt", SHT_PROGBITS,
+                  SHF_ALLOC | SHF_EXECINSTR);
+    s1->plt->sh_entsize = 4;
     }

Gnah!  First the whole conditional statements aren't indended anymore, and 
second the arguments to the new_section call aren't aligned.

This is all quite obvious and terrible, and I could go on and on just 
citing from the diffs.  Have you even _looked_ at your patches before 
committing them?  Fix all of this right away, otherwise we have to revert 
the whole series.

Also there's another argument against generally doing such code 
reformattings: git blame is disturbed now, all lines that you've 
reindended now show your commit as the one to blame, which is useless 
information.  Unfortunately that's a damage that can't be undone now 
anymore.

Next time you want to do whole-sale code changes please discuss on the 
mailing list before doing so, there might be reasons for the status quo 
that you're unaware of, like in this case.

Sigh.


Ciao,
Michael.
P.S: some of the reindendation changes look like as if you've replaced 
leading tabs with four spaces.  That would have been wrong, tabs are eight 
spaces.  If this is the case, fix your editor settings.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]