Re: [PATCH 4/5] ide-cd: shorten lines longer than 80 columns

On Thu, Feb 28, 2008 at 12:01:43AM +0100, Bartlomiej Zolnierkiewicz wrote:

On Wednesday 27 February 2008, Borislav Petkov wrote:

quoted

On Wed, Feb 27, 2008 at 10:18:50PM +0100, Bartlomiej Zolnierkiewicz wrote:

quoted

On Wednesday 27 February 2008, Borislav Petkov wrote:

quoted

Signed-off-by: Borislav Petkov <redacted>

interdiff for the merged version

[...]

quoted

quoted

diff -u b/drivers/ide/ide-cd.c b/drivers/ide/ide-cd.c

--- b/drivers/ide/ide-cd.c
+++ b/drivers/ide/ide-cd.c

@@ -510,7 +510,7 @@
 	default:
 		if (!(rq->cmd_flags & REQ_QUIET))
 			printk(KERN_INFO "ide-cd: cmd 0x%x timed out\n",
-				rq->cmd[0]);
+					 rq->cmd[0]);

why do you push the rq->cmd[0] thingy back here. Shouldn't it be aligned
with the opening brace as the zillion others so far?

IDE is quite consistent with the above style but maybe it is just me being
used to it.  I could revert this change if you feel strong about it...

not at all, i'm not that concerned with codestyle issues as long as it is mostly
readable :). The only issue of importance is consistency along all the source
files so i'll try to keep up with the rest of ide.

quoted

quoted

@@ -819,8 +819,8 @@
 static void restore_request(struct request *rq)
 {
 	if (rq->buffer != bio_data(rq->bio)) {
-		sector_t n = (rq->buffer - (char *) bio_data(rq->bio)) /
-				SECTOR_SIZE;
+		sector_t n =
+			(rq->buffer - (char *)bio_data(rq->bio)) / SECTOR_SIZE;

i must say, lines like this one always look ugly, no matter the formatting
tricks.

For this case there is a simple solution: fix ide-cd to use hwif->sg_table
instead of having it to walk rq->bio's and the whole function will vanish. ;-)

Is this hwif->sg_table walk something similar to what it is being done in
ide_build_sglist() or ide_build_dmatable()?

[ IOW We are putting way too much time into these coding style fixes and as
  the old code is replaced with the new one the coding style improves itself. ]

Exactly my thoughts. On the one hand, it is important to clean up the code
before you stare at it for a magnitude of hours looking for bugs and actually this
cleanup was supposed to be really brief, to begin with, but you know how things
really work out in reality :). On the other hand, there are places in ide-cd that
are just horrible and need rewriting, besides API changes.

On a different note, i'm working on ripping out pipelining from ide-tape
since this'll make ATAPI unification a lot easier. Do you happen to have figured
out what this tape->merge_stage is supposed to do, i know it is a pipeline stage
and in all r/w requests the tape->merge_stage->bh is the buffer that
receives/sends the data to/from the device but still it looks really strange.
What i've done so far is remove the pipeline control algos and have kept the
tape->merge_stage for simplicity. In a different cycle i'll have to kick it out
too replacing it with a simple data buffer.

Any ideas?

Thanks.

-- 
Regards/Gruß,
    Boris.