linux-kernel - Re: [PATCH] [6/8] Account overlapped mappings in end_pfn

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 11 Feb 2008 14:55:35 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Andi Kleen <ak@...e.de>
Cc:	ying.huang@...el.com, tglx@...utronix.de,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [6/8] Account overlapped mappings in end_pfn_map

* Andi Kleen <ak@...e.de> wrote:

> > your patch is also a bit unclean:
> 
> Ok patch with hungarized variables appended.

My comments have nothing to do with "hungarized variables". You used 
clearly unclean and ambigious coding constructs like:

+static unsigned long __meminit
 phys_pmd_update(pud_t *pud, unsigned long address, unsigned long end)
 {
+       unsigned long r;
        pmd_t *pmd = pmd_offset(pud, 0);
        spin_lock(&init_mm.page_table_lock);
-       phys_pmd_init(pmd, address, end);
+       r = phys_pmd_init(pmd, address, end);
        spin_unlock(&init_mm.page_table_lock);
        __flush_tlb_all();
+       return r;
 }

the "r" name is totally unintuitive and the reader of the code is given 
absolutely no idea what happens here.

The cleanliness rules about descriptive variable names are obvious, 
necessary and well respected throughout the kernel, by all other kernel 
contributors i know. For years you were allowed to merge such patches. 
You should really (re-)learn and accept the rules that all other kernel 
contributors have been following for years.

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/