Re: [PATCH 2/3] ARM: cache-uniphier: refactor jump label to follow coding style guideline

From	Russell King - ARM Linux <linux@armlinux.org.uk>
Newsgroups	linux.kernel
Subject	Re: [PATCH 2/3] ARM: cache-uniphier: refactor jump label to follow coding style guideline
Date	2016-11-04 14:40 +0100
Message-ID	<szMSW-2gE-11@gated-at.bofh.it> (permalink)
References	<szLap-181-1@gated-at.bofh.it> <szLap-181-13@gated-at.bofh.it> <szLN7-1Em-9@gated-at.bofh.it> <szMga-1OG-15@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

On Fri, Nov 04, 2016 at 09:50:56PM +0900, Masahiro Yamada wrote:
> Hi Russell,
> 
> 2016-11-04 21:23 GMT+09:00 Russell King - ARM Linux <linux@armlinux.org.uk>:
> > On Fri, Nov 04, 2016 at 08:43:35PM +0900, Masahiro Yamada wrote:
> >> Documentation/CodingStyle recommends to use label names which say
> >> what the goto does or why the goto exists.
> >>
> >> Just in case, split it up into three labels because the CodingStyle
> >> says "one err bugs" is a common type of bug (although, I do not
> >> believe the current code includes such a bug).
> >
> > However, this has the effect of making the code unnecessarily more
> > complicated, which is a bad thing.  Avoiding unnecessary code
> > complexity wins over style rules.
> 
> 
> I thought this patch is stupid, but makes the code more straight-forward;
> the failure path only calls really needed iounmap/kfree()
> without exploiting that NULL input makes them no-op.

... while making it more fragile, because we're going back to a
situation where the right places need to jump to the right label
in the cleanup, so that the right functions are called.

This is a backwards step.

The reason that iounmap() and kfree() check for NULL pointers is to
allow the cleanup paths to be simple, and that's very important as
many cleanup paths are simply _not_ tested.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Thread

[PATCH 0/3] ARM: cache-uniphier: slight cleanups and trivial coding style fix Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-04 12:50 +0100
  [PATCH 2/3] ARM: cache-uniphier: refactor jump label to follow coding style guideline Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-04 12:50 +0100
    Re: [PATCH 2/3] ARM: cache-uniphier: refactor jump label to follow  coding style guideline Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-04 13:30 +0100
      Re: [PATCH 2/3] ARM: cache-uniphier: refactor jump label to follow  coding style guideline Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-04 14:00 +0100
        Re: [PATCH 2/3] ARM: cache-uniphier: refactor jump label to follow  coding style guideline Russell King - ARM Linux <linux@armlinux.org.uk> - 2016-11-04 14:40 +0100
          Re: [PATCH 2/3] ARM: cache-uniphier: refactor jump label to follow  coding style guideline Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-05 04:20 +0100
  [PATCH 1/3] ARM: cache-uniphier: call kzalloc() after DT property parsing Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-04 12:50 +0100
  [PATCH 3/3] ARM: cache-uniphier: clean up active way setup code Masahiro Yamada <yamada.masahiro@socionext.com> - 2016-11-04 12:50 +0100

csiph-web