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

Re: [Xen-devel] [PATCH 03/14] AMD/IOMMU: Fix multiple reference counting errors

>>> On 23.11.18 at 17:03, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 23/11/2018 08:23, Jan Beulich wrote:
>>>>> On 22.11.18 at 18:46, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 22/11/2018 14:51, Jan Beulich wrote:
>>>>> @@ -220,12 +219,18 @@ void guest_iommu_add_ppr_log(struct domain *d, u32 
>>>>> entry[])
>>>>>      unmap_domain_page(log_base);
>>>>>      guest_iommu_deliver_msi(d);
>>>>> +
>>>>> +out:
>>>> Please indent by at least one blank (same further down).
>>> I thought you've said that you're no longer using an obsolete version of
>>> diff.
>> I don't think I've ever said so, and I don't think it matters much what
>> I personally use.
> The only reason we have this quirk of a coding style is because you
> insisted when David was putting together CODING_STYLE, and the reason
> you gave was to work around versions of `diff -u` which mistook the case
> label for the function name when creating the unified header.  v1 of
> David's patch very specifically didn't have the one space, because he
> was actually trying to get rid of that, as it was (and still is)
> completely unexpected for new developers, and a frequent source of curt
> review responses.

Personally I find it undesirable to have anything other that file
scope things to start in column 1. Of course, as you're likely
aware, I'm an opponent of labels and goto-s altogether, and
probably the former dislike is sort of a result of the latter.

> If this is no longer a concern, can I suggest we update CODING_STYLE. 

Even the version of diff on my main most modern system (3.6)
gets this wrong, so I'd prefer if we could stick to indented labels.
I continue to be of the opinion that the use of git should not be
a requirement for developers; plain diff and patch ought to
suffice for people to produce acceptable patches.

I'd like to note though that ./CODING_STYLE says nothing either
way afaics, so I also have no basis to _insist_ on this style. I've
just sent a patch to rectify this.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.