All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] Revert "kconfig: qconf: don't show goback button on splitMode"
@ 2020-08-01  7:08 Masahiro Yamada
  2020-08-01  7:08 ` [PATCH 2/2] kconfig: qconf: remove wrong ConfigList::firstChild() Masahiro Yamada
  0 siblings, 1 reply; 2+ messages in thread
From: Masahiro Yamada @ 2020-08-01  7:08 UTC (permalink / raw)
  To: linux-kbuild
  Cc: Mauro Carvalho Chehab, Masahiro Yamada, Maxim Levitsky, linux-kernel

This reverts commit cc1c08edccaf5317d99a17a3231fe06381044e83.

Maxim Levitsky reports 'make xconfig' crashes since that commit
(https://lkml.org/lkml/2020/7/18/411)

Or, the following is simple test code that makes it crash:

    menu "Menu"

    config FOO
            bool "foo"
            default y

    menuconfig BAR
            bool "bar"
            depends on FOO

    endmenu

Select the Split View mode, and double-click "bar" in the right window,
then you will see Segmentation fault.

When 'last' is not set for symbolMode, the following code in
ConfigList::updateList() calls firstChild().

  item = last ? last->nextSibling() : firstChild();

However, ConfigList::firstChild() does not seem to return a (ConfigItem *)
capable pointer, which seems another bug.

Moreover, we should reconsider whether hiding the goback icon is the
right thing to do.

In the following test code, the Split View shows "Menu2" and "Menu3" in
the right window. You can descend into "Menu3", but there is no way to
ascend back to "Menu2" from "Menu3".

    menu "Menu1"

    config FOO
            bool "foo"
            default y

    menu "Menu2"
            depends on FOO

    menu "Menu3"

    config BAZ
            bool "baz"

    endmenu

    endmenu

    endmenu

It is true that the goback button is currently not functional due to yet
another bug, but hiding the problem is not the right way to go.

Anyway, Segmentation fault is rather fatal. Revert the offending commit
for now, and we should find the right solution.

Reported-by:  Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kconfig/qconf.cc | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index 4a616128a154..75c7b57fd7a9 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -437,10 +437,9 @@ void ConfigList::updateList(ConfigItem* item)
 	if (rootEntry != &rootmenu && (mode == singleMode ||
 	    (mode == symbolMode && rootEntry->parent != &rootmenu))) {
 		item = (ConfigItem *)topLevelItem(0);
-		if (!item && mode != symbolMode) {
+		if (!item)
 			item = new ConfigItem(this, 0, true);
-			last = item;
-		}
+		last = item;
 	}
 	if ((mode == singleMode || (mode == symbolMode && !(rootEntry->flags & MENU_ROOT))) &&
 	    rootEntry->sym && rootEntry->prompt) {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [PATCH 2/2] kconfig: qconf: remove wrong ConfigList::firstChild()
  2020-08-01  7:08 [PATCH 1/2] Revert "kconfig: qconf: don't show goback button on splitMode" Masahiro Yamada
@ 2020-08-01  7:08 ` Masahiro Yamada
  0 siblings, 0 replies; 2+ messages in thread
From: Masahiro Yamada @ 2020-08-01  7:08 UTC (permalink / raw)
  To: linux-kbuild; +Cc: Mauro Carvalho Chehab, Masahiro Yamada, linux-kernel

This function returns the first child object, but the returned pointer
is not compatible with (ConfigItem *).

Commit cc1c08edccaf ("kconfig: qconf: don't show goback button on
splitMode") uncovered this because using the pointer from this function
would make qconf crash. (https://lkml.org/lkml/2020/7/18/411)

This function does not work. Do not use it.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kconfig/qconf.cc | 2 +-
 scripts/kconfig/qconf.h  | 4 ----
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/scripts/kconfig/qconf.cc b/scripts/kconfig/qconf.cc
index 75c7b57fd7a9..80f042a337c2 100644
--- a/scripts/kconfig/qconf.cc
+++ b/scripts/kconfig/qconf.cc
@@ -443,7 +443,7 @@ void ConfigList::updateList(ConfigItem* item)
 	}
 	if ((mode == singleMode || (mode == symbolMode && !(rootEntry->flags & MENU_ROOT))) &&
 	    rootEntry->sym && rootEntry->prompt) {
-		item = last ? last->nextSibling() : firstChild();
+		item = last ? last->nextSibling() : nullptr;
 		if (!item)
 			item = new ConfigItem(this, last, rootEntry, true);
 		else
diff --git a/scripts/kconfig/qconf.h b/scripts/kconfig/qconf.h
index fb9e9729266f..5eeab4a8bb43 100644
--- a/scripts/kconfig/qconf.h
+++ b/scripts/kconfig/qconf.h
@@ -92,10 +92,6 @@ public slots:
 	{
 		return this;
 	}
-	ConfigItem* firstChild() const
-	{
-		return (ConfigItem *)children().first();
-	}
 	void addColumn(colIdx idx)
 	{
 		showColumn(idx);
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2020-08-01  7:09 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-01  7:08 [PATCH 1/2] Revert "kconfig: qconf: don't show goback button on splitMode" Masahiro Yamada
2020-08-01  7:08 ` [PATCH 2/2] kconfig: qconf: remove wrong ConfigList::firstChild() Masahiro Yamada

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.