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

[bhe at redhat.com]

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.

> 
> >
> > 	/*
> > 	 * This method checks that vmlinux and vmcore are same kernel version.
> >@@ -706,6 +710,9 @@ get_kernel_version(char *release)
> > 		MSG("The kernel version is not supported.\n");
> > 		MSG("The created dumpfile may be incomplete.\n");
> > 	}
> >+
> >+	done = 1;
> >+
> > 	return version;
> > }
> >
> >@@ -9062,6 +9069,22 @@ int is_crashkernel_mem_reserved(void)
> >         return !!crash_reserved_mem_nr;
> > }
> >
> >+static int get_page_offset()
> >+{
> >+#ifdef __x86_64__
> >+	struct utsname utsname;
> >+	if (uname(&utsname)) {
> >+		ERRMSG("Cannot get name and information about current kernel : %s", strerror(errno));
> >+		return FALSE;
> >+	}
> >+
> >+	info->kernel_version = get_kernel_version(utsname.release);
> >+	get_versiondep_info_x86_64();
> >+#endif /* x86_64 */
> 
> You should replace get_versiondep_info_x86_64() with get_versiondep_info()
> to get rid of #ifdef.
> #ifdef is messy, I don't want to use it if possible.

Sure, will do.

> 
> 
> Thanks
> Atsushi Kumagai.
> 
> >+
> >+	return TRUE;
> >+}
> >+
> > static struct option longopts[] = {
> > 	{"split", no_argument, NULL, OPT_SPLIT},
> > 	{"reassemble", no_argument, NULL, OPT_REASSEMBLE},
> >--
> >1.8.5.3
> 
> _______________________________________________
> kexec mailing list
> kexec at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec