linux-kernel - Re: [PATCH 1/2]Trivial patch: to solve indentation warnings in amba-clcd.c driver

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]

Date:	Fri, 13 Feb 2015 03:59:56 -0800
From:	Joe Perches <joe@...ches.com>
To:	Russell King - ARM Linux <linux@....linux.org.uk>
Cc:	Parmeshwr Prasad <parmeshwr_prasad@...l.com>,
	plagnioj@...osoft.com, tomi.valkeinen@...com, trivial@...nel.org,
	linux-fbdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2]Trivial patch: to solve indentation warnings in
 amba-clcd.c driver

On Fri, 2015-02-13 at 11:35 +0000, Russell King - ARM Linux wrote:
> On Fri, Feb 13, 2015 at 06:21:33AM -0500, Parmeshwr Prasad wrote:
> > @@ -288,7 +288,7 @@ static int clcdfb_set_par(struct fb_info *info)
> >         struct clcd_regs regs;
> > 
> >         fb->fb.fix.line_length = fb->fb.var.xres_virtual *
> > -                                fb->fb.var.bits_per_pixel / 8;
> > +           fb->fb.var.bits_per_pixel / 8;
> 
> NAK on this one.  The code as it stood before is much clearer since
> we align the expression with the start of it on the preceding line.

I agree with all of what Russell wrote, but maybe this;

> > @@ -717,7 +716,9 @@ static int clcdfb_of_vram_setup(struct clcd_fb *fb)
> >                 return -ENOMEM;
> > 
> >         fb->fb.fix.smem_start = of_translate_address(memory,
> > -                       of_get_address(memory, 0, &size, NULL));
> > +                                                    of_get_address(memory, 0,
> > +                                                                   &size,
> > +                                                                   NULL));
> 
> Thi sis the exception to the rule - where scrunching an expression so that
> it takes multiple lines because of lack of right-hand space is not on.
> The former version was a lot better.

Perhaps this could be better as:

	fb->fb.fix.smem_start =
		 of_translate_address(memory,
				      of_get_address(memory, 0, &size, NULL));

But sometimes using multiple statements instead of
embedding function calls as arguments can be simpler
and more intelligible for the reader.

	__be32 addr;

	...

	addr = of_get_address(memory, 0, &size, NULL);
	fb->fb.fix.smem_start = of_translate_address(memory, addr);



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/