Re: [RFC 02/15] PM / Hibernate: snapshot cleanup

Rafael J. Wysocki
2010-03-24T22:35:36Z

On Tuesday 23 March 2010, Jiri Slaby wrote:
> From: Jiri Slaby <jirislaby@gmail.com>
>
> Remove support of reads with offset. This means snapshot_read/write_next
> now does not accept count parameter.
>
> /dev/snapshot handler is converted to simple_read_from_buffer/simple_write_to_buffer.

Makes sense.

One coding style comment, though.

> Signed-off-by: Jiri Slaby <jslaby@suse.cz>
> Cc: Nigel Cunningham <ncunningham@crca.org.au>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> ---
> kernel/power/power.h | 18 +-----
> kernel/power/snapshot.c | 145 ++++++++++++++++++-----------------------------
> kernel/power/swap.c | 8 +-
> kernel/power/user.c | 39 ++++++++-----
> 4 files changed, 88 insertions(+), 122 deletions(-)
>
...
> @@ -159,13 +160,17 @@ static ssize_t snapshot_read(struct file *filp, char __user *buf,
> res = -ENODATA;
> goto Unlock;
> }
> - res = snapshot_read_next(&data->handle, count);
> - if (res > 0) {
> - if (copy_to_user(buf, data_of(data->handle), res))
> - res = -EFAULT;
> - else
> - *offp = data->handle.offset;
> - }
> + if (!pg_offp) { /* on page boundary? */
> + res = snapshot_read_next(&data->handle);
> + if (res <= 0)
> + goto Unlock;
> + } else
> + res = PAGE_SIZE - pg_offp;

The official kernel coding style is to put single instructions under if/else
like this in braces if the other branch of the if/else is multiline (and
therefore naturally in braces). So please do:

+ if (!pg_offp) { /* on page boundary? */
+ res = snapshot_read_next(&data->handle);
+ if (res <= 0)
+ goto Unlock;
+ } else {
+ res = PAGE_SIZE - pg_offp;
+ }

and analogously wherever applicable.

Thanks,
Rafael