[U-Boot] [PATCH] common/cmd_ide.c: Coding style cleanup (Lindent)

[Stefan Roese]

Hi Wolfgang,

On Wednesday 13 May 2009 13:32:08 Wolfgang Denk wrote:
> > Not sure if this patch will be accepted. It's probably not so easy to
> > understand the history of changes in this file with this coding style
> > cleanup. But the current condition is so bad that I think it's worth
> > the change.
>
> Another argument against the patch is that it makes the code in some
> places more ugly...
>
> > -const static pio_config_t pio_config_ns [IDE_MAX_PIO_MODE+1] =
> > -{
> > -    /*	Setup  Length  Hold  */
> > -	{ 70,	165,	30 },		/* PIO-Mode 0, [ns]	*/
> > -	{ 50,	125,	20 },		/* PIO-Mode 1, [ns]	*/
> > -	{ 30,	101,	15 },		/* PIO-Mode 2, [ns]	*/
> > -	{ 30,	 80,	10 },		/* PIO-Mode 3, [ns]	*/
> > -	{ 25,	 70,	10 },		/* PIO-Mode 4, [ns]	*/
> > +const static pio_config_t pio_config_ns[IDE_MAX_PIO_MODE + 1] = {
> > +	/*  Setup  Length  Hold  */
> > +	{70, 165, 30},		/* PIO-Mode 0, [ns]     */
> > +	{50, 125, 20},		/* PIO-Mode 1, [ns]     */
> > +	{30, 101, 15},		/* PIO-Mode 2, [ns]     */
> > +	{30, 80, 10},		/* PIO-Mode 3, [ns]     */
> > +	{25, 70, 10},		/* PIO-Mode 4, [ns]     */
> >  };
>
> Instead of nice vertically aligned columns the new code is less
> readable.

OK, I'll change this back to the original alignment.

> > -static void  ide_ident (block_dev_desc_t *dev_desc);
> > -static uchar ide_wait  (int dev, ulong t);
> > +static void ide_ident(block_dev_desc_t * dev_desc);
>
> And I really hate the space after the '*' in lines like these,

I don't like then too. I'm wondering why indent didn't do this consistent.

> especially as it's done more or lessa randomly:
> > -static void	atapi_inquiry(block_dev_desc_t *dev_desc);
> > -ulong atapi_read (int device, lbaint_t blknr, ulong blkcnt, void
> > *buffer); +static void atapi_inquiry(block_dev_desc_t * dev_desc);
> > +ulong atapi_read(int device, lbaint_t blknr, ulong blkcnt, void
> > *buffer);
>
> We have "* dev_desc", but "*buffer" ? That's just bogus.
>
> > -	printf ("\nLoading from IDE device %d, partition %d: "
> > -		"Name: %.32s  Type: %.32s\n",
> > -		dev, part, info.name, info.type);
> > +	printf("\nLoading from IDE device %d, partition %d: "
> > +	       "Name: %.32s  Type: %.32s\n", dev, part, info.name, info.type);
>
> Also, reordering the lines here makes the code more difficult to read.
>
> > -	if (ide_dev_desc[dev].block_read (dev, info.start, 1, (ulong *)addr) !=
> > 1) { -		printf ("** Read error on %d:%d\n", dev, part);
> > -		show_boot_progress (-48);
> > +	if (ide_dev_desc[dev].block_read(dev, info.start, 1, (ulong *) addr) !=
> > 1) {
>
> Why is there a space after the cast?

No idea. I'll try to remove all those spaces.

<snip>

> > +U_BOOT_CMD(ide, 5, 1, do_ide,
> > +	   "IDE sub-system",
> > +	   "reset - reset IDE controller\n"
> > +	   "ide info  - show available IDE devices\n"
> > +	   "ide device [dev] - show or set current device\n"
> > +	   "ide part [dev] - print partition table of one or all IDE devices\n"
> > +	   "ide read  addr blk# cnt\n"
> > +	   "ide write addr blk# cnt - read/write `cnt'"
> > +	   " blocks starting at block `blk#'\n"
> > +	   "    to/from memory address `addr'\n");
> > +
> > +U_BOOT_CMD(diskboot, 3, 1, do_diskboot,
> > +	   "boot from IDE device", "loadAddr dev:part\n");
>
> And indentation should be done by TAB - I see no reason to add more
> space here.

Sure. I'll fix it.

> I agree that this file badly needs to be cleaned up, and I even agree
> to a patch like above - but it has to be done more intelligently. The
> resulting file should adhere to the Coding Style requirements, and be
> consistent in style.

OK, I'll try to fix all those issues and resubmit a new version.

Thanks.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================