[lkml]   [2009]   [Apr]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 4/5] ide: Add support for TRIM

Matthew Wilcox wrote:

>>> +static int idedisk_prepare_discard(struct request_queue *q, struct
>>>request *rq,
>>>+ struct bio *bio)

>> Weird indentation.

> Not at all. New line, so tab the next line over as far as it will go
> without falling off the 80-column limit. It's fairly common.

> To quote Documentation/CodingStyle:

> Statements longer than 80 columns will be broken into sensible chunks.
> Descendants are always substantially shorter than the parent and are placed
> substantially to the right. The same applies to function headers with a long
> argument list. Long strings are as well broken into shorter strings. The
> only exception to this is where exceeding 80 columns significantly increases
> readability and does not hide information.

That's all clear, it's just that the second line was overly indented, at
least to my taste. :-)

>>>+ ide_task_t *task;

>> This patch is already obsolete as 'ide_task_t' is gone. Use 'struct
>>ide_cmd' instead.

> Thanks, fixed.

>>>+ unsigned size;
>>>+ struct page *page = alloc_page(GFP_KERNEL);

>> Missing empty line after the declaration block...

> Empty line not necessary.

Usually it's there.

>>>+ task->tf_flags = IDE_TFLAG_LBA48 | IDE_TFLAG_OUT_HOB |

>> The last 3 flags are going to be obsoleted too...

> So if I remove them today, the command will still work?

s/obsoleted/renamed and moved to another other field/ -- I'm going to
submit a patchset refactoring 'struct ide_cmd and 'struct ide-taskfile' at last...

MBR, Sergei

 \ /
  Last update: 2009-04-02 18:41    [W:0.250 / U:1.924 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site