[Patch v3 3/7] preparation functions for parsing vmcoreinfo

[Atsushi Kumagai]

>On 08/12/14 at 05:46pm, Baoquan He wrote:
>> On 08/01/14 at 07:12am, Atsushi Kumagai wrote:
>> > >diff --git a/makedumpfile.c b/makedumpfile.c
>> > >index 220570e..78aa7a5 100644
>> > >--- a/makedumpfile.c
>> > >+++ b/makedumpfile.c
>> > >@@ -681,6 +681,10 @@ get_kernel_version(char *release)
>> > > 	int32_t version;
>> > > 	long maj, min, rel;
>> > > 	char *start, *end;
>> > >+	static int done = 0;
>> > >+
>> > >+	if (done)
>> > >+		return info->kernel_version;
>> >
>> > This function just convert the argument as string into
>> > a number, it shouldn't be affected by external factors.
>> >
>> > You should use info->kernel_version in the caller side if
>> > you want to avoid duplicate calling of this function, but
>> > I think it's unnecessary since this function is small.
>>
>> In show_mem_usage() implementaion, the page_offset is needed before
>> initial() calling because the dumpable elf program loads have ot be
>> prepared before that. However in current commited code, the page_offset
>> is got in initial() when call check_release().
>>
>> So I have to get it in advance by this way. Then the
>> get_kernel_version() can be reused in this way. Anyway, by this I
>> needn't change the code in initial().
>>
>> If use info->kernel_version directly before initial() calling, it's
>> still zero.
>>
>
>I add the static variable "done" just because it always print below
>warning message twice, this makes me uncomfortable. Otherwise
>get_kernel_version() can be called any time no matter how many times it
>has been called if it's only a converting utility function.
>
>The kernel version is not supported.
>The created dumpfile may be incomplete.

Yeah, I understand the reason.
I agree with your idea, but I think it's better to check the
kernel_version directly like:

	if (info->kernel_version)
		return info->kernel_version;


Thanks
Atsushi Kumagai

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