Re: [PATCH v2 2/5] Add tools for reading and writing from splitblock table

[HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>]

From: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
Subject: [PATCH v2 2/5] Add tools for reading and writing from splitblock table
Date: Mon, 13 Oct 2014 17:34:23 +0800

> The function added in this patch, is used for writing and reading value
> from the char array in struct SplitBlock.
> 
> Signed-off-by: Qiao Nuohan <qiaonuohan@cn.fujitsu.com>
> Signed-off-by: Zhou Wenjian <zhouwj-fnst@cn.fujitsu.com>
> ---
>  makedumpfile.c |   23 +++++++++++++++++++++++
>  1 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index 95d553c..a8d86f6 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -5708,6 +5708,29 @@ calculate_entry_size(void){
>  	return entry_size;
>  }
>  
> +void
> +write_value_into_splitblock_table(char *splitblock_inner, unsigned long long content)

This line exceeds 80 characters. The cause would be the name of
function arguments are too long.

How about this?

void
write_value_into_splitblock_table(char *entry, unsigned long long value)


> +{
> +	char temp;
> +	int i=0;

Please add linebreak.

Please add spaces between assignment operator.

int i = 0;

> +	while (i++ < splitblock->entry_size) {
> +		temp = content & 0xff;
> +		content = content >> BITPERBYTE;
> +		*splitblock_inner++ = temp;

Please write increment operator in a separate line.

                *splitblock_inner = temp;
                *splitblock_inner++;

> +	}
> +}
> +unsigned long long
> +read_value_from_splitblock_table(char *splitblock_inner)
> +{
> +	unsigned long long ret = 0;

How about value instead of ret? In addition to the above comment, this
also leads to contrasting consistency between the two helper
functions.

> +	int i;

Please add a linebreak.

> +	for (i = splitblock->entry_size; i > 0; i--) {
> +		ret = ret << BITPERBYTE;
> +		ret += *(splitblock_inner + i - 1) & 0xff;

The & 0xff is necessary? because splitblock_inner is of type char *.

> +	}
> +	return ret;
> +}
> +
>  mdf_pfn_t
>  get_num_dumpable(void)
>  {
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
Thanks.
HATAYAMA, Daisuke


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec