Re: [PATCH v2 1/1] OvmfPkg : QemuFwCfgLib: Use BusMasterCommandBuffer to map FW_CFG_DMA_ACCESS

[Laszlo Ersek <lersek@redhat.com>]

On 08/04/17 19:08, Jordan Justen wrote:
> On 2017-08-03 15:27:46, Laszlo Ersek wrote:
>> On 08/04/17 00:07, Brijesh Singh wrote:
>>> Hi Laszlo,
>>>
>>> Thanks for the detail review, I will soon send v3 with all your feedback
>>> addressed. I must admit that I have constant struggle with formating issues
>>> in EDKII contributions. While browsing the code, several packages have
>>> code and comment exceeding 79 char. But looking at your previous feedbacks
>>> somehow I was under assumption that comments should be <= 79 chars but code
>>> can exceed 79 char limit. I think my understanding was wrong. I will update
>>> my vi setting to warn me when I exceed 79 char limit.
>>
>> In the coding style, you will find:
>>
>> https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/5_source_files/#51-general-rules
>>
>> ----------
>> 5 Source Files
>> 5.1 General Rules
>> 5.1.1 Lines shall be 120 columns, or less
>>
>> Preferably, limit line lengths to 80 columns or less. When this doesn't
>> leave sufficient space for a good postfix style comment, extend the line
>> to a total of 120 columns. Having some level of uniformity in the
>> expected width of the source is useful for viewing and printing the code.
> 
> I personally don't like this wording. It basically says, 'we prefer
> 80, but if that is too difficult, then 120 is ok'. The only case I've
> ever seen of where code wouldn't fit reasonably in 80 columns, someone
> was essentially putting a spreadsheet into code. (This is not exactly
> a good case to build a code style around.)
> 
>> ----------
>>
>> I stick with 79 chars because they don't wrap in any kind of window
>> sized to 80 columns. (Some terminal emulators / pager programs insert
>> blank lines or wrap unexpectedly when a line is exactly 80 columns.)
>> And, I like to size my windows to 80 columns because I use only one
>> monitor (I dislike using more than one) and with 80 cols/window I can
>> fit two windows side by side conveniently.
> 
> History (punch cards) and personal workflows (fitting multiple
> terminals) are often cited as reasons for the 'eighty column rule',
> but more fundamentally, I prefer this argument:
> 
> https://softwareengineering.stackexchange.com/a/222998
> 
> Essentially, there's a reason newspapers have columns. It is not easy
> for humans to read very long lines of text.

I appreciate that the stackoverflow comment also mentions

"If lines are too short, text becomes hard to read because you must
constantly jump from one line to the next while reading."

> 
> I have another reason why going beyond 80 is not a good idea for code
> that doesn't apply to normal reading. If you need ~120 columns visible
> to view some lines, then most lines will end up having a lot of wasted
> horizontal whitespace because they can commonly fit into 80 columns.

One reason I dislike the "all arguments on separate lines" rule is just
this -- it wastes perfectly good horizontal space, and eats up precious
vertical space.

We have a patch submission process for our documents now, but I don't
think maintainers of other Pkgs would bother with following and
enforcing a stricter 80 columns rule, even if we got it codified. :/

Thanks
Laszlo