linux-kernel - Re: [PATCH 03/47] block-rbd: Adjust the position of a jump label in rbd_header_from

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]

Message-ID: <20160913163626.436629af@endymion>
Date:   Tue, 13 Sep 2016 16:36:26 +0200
From:   Jean Delvare <jdelvare@...e.de>
To:     Ilya Dryomov <idryomov@...il.com>
Cc:     Jonathan Corbet <corbet@....net>,
        Ceph Development <ceph-devel@...r.kernel.org>,
        Alex Elder <elder@...nel.org>, Sage Weil <sage@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        kernel-janitors@...r.kernel.org,
        Julia Lawall <julia.lawall@...6.fr>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH 03/47] block-rbd: Adjust the position of a jump label in
 rbd_header_from_disk()

Hi Ilya,

Thanks for adding me.

On Tue, 13 Sep 2016 11:16:13 +0200, Ilya Dryomov wrote:
> On Tue, Sep 13, 2016 at 10:12 AM, SF Markus Elfring
> <elfring@...rs.sourceforge.net> wrote:
> >>> @@ -1064,7 +1064,7 @@ static int rbd_header_from_disk(struct rbd_device *rbd_dev,
> >>>         header->snap_sizes = snap_sizes;
> >>>
> >>>         return 0;
> >>> -out_2big:
> >>> + out_2big:
> >>>         ret = -EIO;
> >>>         kfree(snap_sizes);
> >>>   free_names:
> > …
> >> Can you point where this current convention is documented?
> >
> > Yes.
> > https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/Documentation/CodingStyle?id=865a1caa4b6b886babdd9d67e7c3608be4567a51
> 
> Huh.  That patch is not in Linus' tree.
> 
> >
> > Do you find the software update "CodingStyle: Clarify and complete chapter 7" interesting?
> >
> >
> >> Certainly not in CodingStyle, AFAICT...
> >
> > I suggest to look at the current version once more.
> >
> >
> >> I know some people prefer a single space in there because it makes
> >> "diff -p" work better, but nowadays with "git diff" this argument is
> >> pretty moot.
> >
> > Would you like to discuss the corresponding software evolution a bit more?
> 
> Jon, could you please yank 865a1caa4b6b ("CodingStyle: Clarify and
> complete chapter 7") from your linux-next branch or at least change "It
> is advised to indent labels" to something less stronger?  It hasn't
> even hit mainline yet and we are already getting spammed.

The problem isn't the documentation update nor whether you or me like a
space before labels or not. The problem is Markus Elfring. The guy just
spend his time flooding maintainers with unneeded changes they never
asked for. Ignore him and you'll be much better. If he was not flooding
you with this, he would find something else :-(

When I wrote "It is advised to indent labels with one space", I never
meant that all the existing code should be converted that way. I
expressed a preference, and provided a rationale for this preference.
After that, an advice is just that: an advice.

> Looks like 9 out of 10 labels are not indented
> 
> $ git grep '^[a-z0-9]\+:' -- *.c | wc -l
> 27945
> $ git grep '^ [a-z0-9]\+:' -- *.c | wc -l
> 2925

Your regexps are wrong ;-) but the ratio is correct.

> so I'd say that's a bad advise as far as consistency goes, and the
> "diff -p" argument is pretty moot nowadays.

It wasn't moot when I sent the documentation update patch. Or why would
you think it was? "git diff", by default, behaves exactly the same as
"diff -p" with regards to unindented labels (i.e. it doesn't handle
them properly.)

However, since then the issue was discussed somewhere else:
https://lkml.org/lkml/2016/9/5/214

As you can see, alternatives to indenting labels with one space were
found. Therefore you will soon be correct saying "the diff -p argument
is pretty moot." As soon as my patch hits mainline, actually. Which
shouldn't take too long as Andrew Morton picked it 4 days ago.

Once this happens, I'm fine with CodingStyle being updated again to
reflect the current situation.

Hope it clarifies,
-- 
Jean Delvare
SUSE L3 Support