[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Crash-utility] [ANNOUNCE][RFC] gcore extension module: user-mode process core dump



Hello Dave,

I've just fixed gcore. The patset is attached to this mail.

Could you review and apply them if okay?

Primary changes are:
 - no build process on unsupported architectures, and
 - fix verbose handling: -v7 is now handled correctly.

In particular, I've just emproyed the way you suggested as below:

> 
> Or you could just catch it in the gcore.mk by doing something like this:
> 
>   ARCH=UNSUPPORTED
>   ifeq ($(shell arch), x86_64)
>     ARCH=SUPPORTED
>   endif
>   ifeq ($(shell arch), i686)
>     ARCH=SUPPORTED
>   endif
> 
>   all: gcore.so
> 
>   gcore.so: gcore.c
>           @if [ ${ARCH} = "UNSUPPORTED"  ]; then \
>                   echo "gcore: architecture not supported"; else \
>           echo "do build here..."; fi;

I confirmed this works well on IA64.

Thanks,
HATAYAMA Daisuke
>From 686a13cace0f5f0276335fb7a3210ae258dd052f Mon Sep 17 00:00:00 2001
From: HATAYAMA Daisuke <d hatayama jp fujitsu com>
Date: Tue, 25 Jan 2011 11:39:10 +0900
Subject: [PATCH 1/5] verbose: fix wrong comparison with verbose max level

'>=' should have been '>'.

Signed-off-by: HATAYAMA Daisuke <d hatayama jp fujitsu com>
---
 src/libgcore/gcore_verbose.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/libgcore/gcore_verbose.c b/src/libgcore/gcore_verbose.c
index afc9163..172a3d9 100644
--- a/src/libgcore/gcore_verbose.c
+++ b/src/libgcore/gcore_verbose.c
@@ -33,7 +33,7 @@ void gcore_verbose_set_default(void)
 
 int gcore_verbose_set(ulong level)
 {
-	if (level >= VERBOSE_MAX_LEVEL)
+	if (level > VERBOSE_MAX_LEVEL)
 		return FALSE;
 	gvd->level = level;
 	if (gvd->level & VERBOSE_NONQUIET)
-- 
1.7.3.5

>From a805713e7a8ae80f41dd7f70ffb42fafb9b5fc1b Mon Sep 17 00:00:00 2001
From: HATAYAMA Daisuke <d hatayama jp fujitsu com>
Date: Tue, 25 Jan 2011 11:42:20 +0900
Subject: [PATCH 2/5] verbose: Add test cases

Add test cases for functions used for gcore verbose features.

Signed-off-by: HATAYAMA Daisuke <d hatayama jp fujitsu com>
---
 src/gcore.c                  |    1 +
 src/libgcore/gcore_defs.h    |    1 +
 src/libgcore/gcore_verbose.c |   52 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 54 insertions(+), 0 deletions(-)

diff --git a/src/gcore.c b/src/gcore.c
index 45bf5a9..f72bd4f 100644
--- a/src/gcore.c
+++ b/src/gcore.c
@@ -500,6 +500,7 @@ void cmd_gcore_test(void)
 	TEST_MODULE(gcore_x86_test);
 	TEST_MODULE(gcore_coredump_table_test);
 	TEST_MODULE(gcore_dumpfilter_test);
+	TEST_MODULE(gcore_verbose_test);
 
 	if (!message)
 		fprintf(fp, "All test cases are successfully passed\n");
diff --git a/src/libgcore/gcore_defs.h b/src/libgcore/gcore_defs.h
index 4c20f07..ed5b899 100644
--- a/src/libgcore/gcore_defs.h
+++ b/src/libgcore/gcore_defs.h
@@ -806,6 +806,7 @@ extern int tests_run;
 extern char *gcore_x86_test(void);
 extern char *gcore_coredump_table_test(void);
 extern char *gcore_dumpfilter_test(void);
+extern char *gcore_verbose_test(void);
 
 #endif
 
diff --git a/src/libgcore/gcore_verbose.c b/src/libgcore/gcore_verbose.c
index 172a3d9..cd955d1 100644
--- a/src/libgcore/gcore_verbose.c
+++ b/src/libgcore/gcore_verbose.c
@@ -52,3 +52,55 @@ ulong gcore_verbose_error_handle(void)
 {
 	return gvd->error_handle;
 }
+
+#ifdef GCORE_TEST
+
+char *gcore_verbose_test(void)
+{
+	int test;
+
+	gcore_verbose_set_default();
+	test = gcore_verbose_set(VERBOSE_PROGRESS);
+	mu_assert("failed to set VERBOSE_PROGRESS", test);
+	test = !!(gcore_verbose_get() & VERBOSE_PROGRESS);
+	mu_assert("VERBOSE_PROGRESS is not set even after set operation", test);
+	test = !!(gcore_verbose_error_handle() & QUIET);
+	mu_assert("error_handle is not set to QUIET", test);
+
+	gcore_verbose_set_default();
+	test = gcore_verbose_set(VERBOSE_NONQUIET);
+	mu_assert("failed to set VERBOSE_NONQUIET", test);
+	test = !!(gcore_verbose_get() & VERBOSE_NONQUIET);
+	mu_assert("VERBOSE_NONQUIET is not set even after set operation", test);
+	test = !!(gcore_verbose_error_handle() & QUIET);
+	mu_assert("error_handle is set to QUIET even if VERBOSE_NONQUIET is set", !test);
+
+	gcore_verbose_set_default();
+	test = gcore_verbose_set(VERBOSE_PAGEFAULT);
+	mu_assert("failed to set VERBOSE_PAGEFAULT", test);
+	test = !!(gcore_verbose_get() & VERBOSE_PAGEFAULT);
+	mu_assert("VERBOSE_PAGEFAULT is not set even after set operation", test);
+	test = !!(gcore_verbose_error_handle() & QUIET);
+	mu_assert("error_handle is not set to QUIET", test);
+
+	gcore_verbose_set_default();
+	test = gcore_verbose_set(VERBOSE_PAGEFAULT | VERBOSE_NONQUIET);
+	mu_assert("failed to set VERBOSE_PAGEFAULT | VERBOSE_NONQUIET", test);
+	test = !!(gcore_verbose_get() & (VERBOSE_PAGEFAULT | VERBOSE_NONQUIET));
+	mu_assert("VERBOSE_PAGEFAULT is not set even after set operation", test);
+	test = !!(gcore_verbose_error_handle() & QUIET);
+	mu_assert("error_handle is not set to QUIET", !test);
+
+	gcore_verbose_set_default();
+	test = gcore_verbose_set(VERBOSE_MAX_LEVEL);
+	mu_assert("VERBOSE_MAX_LEVEL should be valid, but here thought of as invalid.", test);
+
+	gcore_verbose_set_default();
+	test = gcore_verbose_set(VERBOSE_MAX_LEVEL+1);
+	mu_assert("(VERBOSE_MAX_LEVEL+1) should be invalid, but somehow accepted.", !test);
+
+	return NULL;
+}
+
+#endif /* GCORE_TEST */
+
-- 
1.7.3.5

>From ee164aecd981f43d7a9572206f8c3fc735fd1168 Mon Sep 17 00:00:00 2001
From: HATAYAMA Daisuke <d hatayama jp fujitsu com>
Date: Tue, 25 Jan 2011 11:44:59 +0900
Subject: [PATCH 3/5] x86: Remove unused IO_BITMAP_OFFSET

Remove IO_BITMAP_OFFSET. It's unused.

Signed-off-by: HATAYAMA Daisuke <d hatayama jp fujitsu com>
---
 src/libgcore/gcore_x86.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/libgcore/gcore_x86.c b/src/libgcore/gcore_x86.c
index d69b577..6417b1b 100644
--- a/src/libgcore/gcore_x86.c
+++ b/src/libgcore/gcore_x86.c
@@ -816,7 +816,6 @@ static int regset_tls_get(struct task_context *target,
 #define IO_BITMAP_BITS  65536
 #define IO_BITMAP_BYTES (IO_BITMAP_BITS/8)
 #define IO_BITMAP_LONGS (IO_BITMAP_BYTES/sizeof(long))
-#define IO_BITMAP_OFFSET offsetof(struct tss_struct,io_bitmap)
 
 static int
 ioperm_active(struct task_context *target,
-- 
1.7.3.5

>From 13639b33f42c0699a256448a0c82e8025a7e2cdb Mon Sep 17 00:00:00 2001
From: HATAYAMA Daisuke <d hatayama jp fujitsu com>
Date: Wed, 26 Jan 2011 10:00:12 +0900
Subject: [PATCH 4/5] gcore.mk: Add conditional to identify supported and unsupported archtectures

Echo "gcore: architecture not supported" only on unsupported architectures.

Signed-off-by: HATAYAMA Daisuke <d hatayama jp fujitsu com>
---
 src/gcore.mk |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/src/gcore.mk b/src/gcore.mk
index dc24704..08b7fbb 100644
--- a/src/gcore.mk
+++ b/src/gcore.mk
@@ -12,14 +12,18 @@
 # GNU General Public License for more details.
 #
 
+ARCH=UNSUPPORTED
+
 ifeq ($(shell arch), i686)
   TARGET=X86
   TARGET_CFLAGS=-D_FILE_OFFSET_BITS=64
+  ARCH=SUPPORTED
 endif
 
 ifeq ($(shell arch), x86_64)
   TARGET=X86_64
   TARGET_CFLAGS=
+  ARCH=SUPPORTED
 endif
 
 ifeq ($(shell /bin/ls /usr/include/crash/defs.h 2>/dev/null), /usr/include/crash/defs.h)
@@ -49,9 +53,14 @@ GCORE_OFILES = $(patsubst %.c,%.o,$(GCORE_CFILES))
 COMMON_CFLAGS=-Wall -I$(INCDIR) -I./libgcore -fPIC -D$(TARGET)
 
 all: gcore.so
-	
-gcore.so: $(INCDIR)/defs.h gcore.c $(GCORE_OFILES)
-	gcc $(TARGET_CFLAGS) $(COMMON_CFLAGS) -nostartfiles -shared -rdynamic $(GCORE_OFILES) -o gcore.so gcore.c
+
+gcore.so: gcore.c $(INCDIR)/defs.h
+	@if [ $(ARCH) = "UNSUPPORTED"  ]; then \
+		echo "gcore: architecture not supported"; \
+	else \
+		make -f gcore.mk $(GCORE_OFILES) && \
+		gcc $(TARGET_CFLAGS) $(COMMON_CFLAGS) -nostartfiles -shared -rdynamic $(GCORE_OFILES) -o $@ $< ; \
+	fi;
 
 %.o: %.c $(INCDIR)/defs.h
 	gcc $(TARGET_CFLAGS) $(COMMON_CFLAGS) -c -o $@ $<
-- 
1.7.3.5

>From e42380171c78ca621b9df1077728f0838b510fc4 Mon Sep 17 00:00:00 2001
From: HATAYAMA Daisuke <d hatayama jp fujitsu com>
Date: Wed, 26 Jan 2011 11:25:28 +0900
Subject: [PATCH 5/5] test: fix wrongly displaying test results

Introduce a new flag, failed, indicating whether some testcase has
failed, by which successful information is displayed correctly when
all test cases are passed.

Signed-off-by: HATAYAMA Daisuke <d hatayama jp fujitsu com>
---
 src/gcore.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/gcore.c b/src/gcore.c
index f72bd4f..1329e75 100644
--- a/src/gcore.c
+++ b/src/gcore.c
@@ -490,19 +490,22 @@ NULL,
 
 void cmd_gcore_test(void)
 {
+	int failed = FALSE;
 	char *message = NULL;
 
 #define TEST_MODULE(test)					\
 	message = test();					\
-	if (message)						\
-		fprintf(fp, #test ": %s\n", message);
+	if (message) {						\
+		failed = TRUE;					\
+		fprintf(fp, #test ": %s\n", message);		\
+	}
 
 	TEST_MODULE(gcore_x86_test);
 	TEST_MODULE(gcore_coredump_table_test);
 	TEST_MODULE(gcore_dumpfilter_test);
 	TEST_MODULE(gcore_verbose_test);
 
-	if (!message)
+	if (!failed)
 		fprintf(fp, "All test cases are successfully passed\n");
 
 #undef TEST_MODULE
-- 
1.7.3.5


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]