| ||||||||||||||||||||||
Re: [PATCH 2/2] x86, pci: Increase the number of iommus supported to be MAX_IO_APICS v2
* Andrew Morton <akpm@linux-foundation.org> wrote:
> Also we can tweak the code flow and the message to avoid dorky
> 80-column games:
> + printk_once(KERN_ERR "intel-iommu: exceeded %d IOMMUs\n",
> IOMMU_UNITS_SUPPORTED);
Not to mention the use of pr_err():
pr_err("intel-iommu: exceeded %d IOMMUs\n", IOMMU_UNITS_SUPPORTED);
Plus if we defined a proper driver message prefix at the top of
the driver:
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
We could do:
pr_err("Exceeded max %d IOMMUs\n", IOMMU_UNITS_SUPPORTED);
Note, I added 'max', for clarity.
Plus IOMMU_UNITS_SUPPORTED could be renamed to the much shorter
IOMMU_MAX, without a loss of clarity:
pr_err("Exceeded max %d IOMMUs\n", IOMMU_MAX);
So we made that line vastly shorter, and made the human-readable
message actually longer and more expressive.
80 column wraps are almost always not a sign of lack of screen
real estate, but a symptom of lack of thinking.
Thanks,
Ingo
(Log in to post comments)
A sign of lack of thinking? Posted Mar 1, 2012 11:47 UTC (Thu) by rswarbrick (subscriber, #47560) [Link]
... or a sign of using the GTK framework. Function names like gtk_editable_get_selection_bounds (), anyone? :-)
No, 80 column wraps Posted Mar 1, 2012 12:48 UTC (Thu) by lacos (subscriber, #70616) [Link]
are there for people who actually like to compare patches (I understand this may not be characteristic of Linux upstream), or have a bugzilla window / a serial console / a log snippet / a git grep output browser open beside the source window. Are multiple physical displays required to work on the kernel now?
Let me lay out the consistency of the argument for you.
> Now, some people will claim that having 8-character indentations makes
(1) The CodingStyle mandates hardware tabs, visually expanded as 8 spaces. -- This is retarded.
(2) The CodingStyle requires a max (expanded) width of 80 chars. -- This is good, but violated countless times in the kernel source. Which proves that either (1) is wrong indeed, or that kernel developers are "screwed anyway, and should fix [their] program[s]".
(3) The cited message ridicules reasonable requirement (2), and stays silent on wrong requirement (1). It's the polar opposite of what should have been done years ago.
No, 80 column wraps Posted Mar 2, 2012 7:12 UTC (Fri) by aleXXX (subscriber, #2742) [Link]
We settled here for 120 columns limit, with 2 space indentation.
Alone the head of a for-loop even with short variable names is quite long in C++:
for(std::map<int, int>::const_iterator it = m_foo.begin(); it != m_foo.end(); ++)
This is already 84 characters, and only one level indented.
This has so nothing to do with making up overly long names, or being retarded, or whatever.
Alex
No, 80 column wraps Posted Mar 2, 2012 7:23 UTC (Fri) by khim (subscriber, #9252) [Link] Actually this loop should look more like: for (auto &element : m_foo) Plenty of space for more meaningful names... Even if you'll just replace “std::map<int, int>::const_iterator” with “auto” line will fit in 80 characters limit! And it'll be more robust to boot (you can change type of m_foo with something like std::unordered_map - and everything will still compile and work).
No, 80 column wraps Posted Mar 2, 2012 12:37 UTC (Fri) by aleXXX (subscriber, #2742) [Link]
...if you already wanr ro use C++11
No, 80 column wraps Posted Mar 2, 2012 21:47 UTC (Fri) by robbe (guest, #16131) [Link]
I think you misunderstood Ingo. IMHO he argued that, rather than mindlessly breaking up the line at the 80 character boundary, one should make the whole statement fit into 80 characters, all the while making it more concise and to the point.
No, 80 column wraps Posted Mar 3, 2012 11:22 UTC (Sat) by lacos (subscriber, #70616) [Link]
It's possible that I misunderstood his main point, but I do think our priorities differ (if I'm allowed to pack Ingo and my humble self in a single possessive determiner). For me the 80-chars limit must be observed at all costs. If we can compress the expression, that's a big bonus, but if we can't, that's no excuse for breaking the limit.
No, 80 column wraps Posted Mar 6, 2012 11:39 UTC (Tue) by nix (subscriber, #2304) [Link]
So maintaining an 80-column limit is important even at the expense of readability? If you have to rename variables to single-letter names to do it, you'll do so?
Surely not.
80 chars is retarded Posted Mar 8, 2012 6:53 UTC (Thu) by amtota (guest, #4012) [Link]
I've always found it a bit odd when people try to coerce their code into a fixed size width, often losing readability and not using proper variable naming which are much more important IMO.
The argument I often hear is that some want to be able to have two files side by side. Hmm, if only we had a tool that was capable of wrapping text around for us when needed..
I tend to go for 120 chars / 4 per tab, or even 160/4. Let the machines deal with the formatting, and let the code express its intent without artificial constraints.
Re: [PATCH 2/2] x86, pci: Increase the number of iommus supported to be MAX_IO_APICS v2 Posted Mar 8, 2012 10:15 UTC (Thu) by slashdot (guest, #22014) [Link]
Any line limit is idiotic, of course.
Simply write lines as long as you like, and the editors or terminals wrap them.
Or if you want a limit, make it 32760 columns.
|
||||||||||||||||||||||