Skip to contentSkip to conversation

ACPICA: ACPI 2.0, Interpreter: Fix MLC issues by switching to new TermList grammar for table loading

PATCH New Dan Carpenter 2 participants 4 messages Jun 4, 2016 – Jun 6, 2016

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Hello Lv Zheng,

The patch 0bfd2f1ec274: "ACPICA: ACPI 2.0, Interpreter: Fix MLC issues by switching to new TermList grammar for table loading" from May 13, 2016, leads to the following static checker warning:

drivers/acpi/acpica/nsparse.c:139 acpi_ns_execute_table()
error: we previously assumed 'info' could be null (see line 110)

drivers/acpi/acpica/nsparse.c
135 (void)acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
136
137 cleanup:
138 acpi_ut_remove_reference(method_obj);

   139          ACPI_FREE(info->full_pathname);
                          ^^^^^^^^^^^^^^^^^^^

Archetypal one err bug. It's better to unwind in reverse order from how you allacted things and only free things you have allocated. Use more than one label. Also you can use more descriptive label names.

	(void)acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);

free_pathname:

	ACPI_FREE(info->full_pathname);
free_info:
	ACPI_FREE(info);
remove_method:
	acpi_ut_remove_reference(method_obj);

	return_ACPI_STATUS(status);
   140          info->full_pathname = NULL;
   141          ACPI_FREE(info);
   142          return_ACPI_STATUS(status);

143 }

regards,
dan carpenter

#1 lore raw

Hi, Dan

Thanks for the report.
I'll double check it with C=2 enabled build.

Best regards
-Lv

Quoted 48 lines

From: [email protected] [mailto:linux-acpi- [email protected]] On Behalf Of Dan Carpenter
Subject: re: ACPICA: ACPI 2.0, Interpreter: Fix MLC issues by switching to new TermList grammar for table loading

Hello Lv Zheng,

The patch 0bfd2f1ec274: "ACPICA: ACPI 2.0, Interpreter: Fix MLC issues by switching to new TermList grammar for table loading" from May 13, 2016, leads to the following static checker warning:

drivers/acpi/acpica/nsparse.c:139 acpi_ns_execute_table()
error: we previously assumed 'info' could be null (see line 110)

drivers/acpi/acpica/nsparse.c

   135          (void)acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
   136
   137  cleanup:
   138          acpi_ut_remove_reference(method_obj);
   139          ACPI_FREE(info->full_pathname);
                          ^^^^^^^^^^^^^^^^^^^

Archetypal one err bug. It's better to unwind in reverse order from how you allacted things and only free things you have allocated. Use more than one label. Also you can use more descriptive label names.

	(void)acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);

free_pathname:

	ACPI_FREE(info->full_pathname);
free_info:
	ACPI_FREE(info);
remove_method:
	acpi_ut_remove_reference(method_obj);

	return_ACPI_STATUS(status);
   140          info->full_pathname = NULL;
   141          ACPI_FREE(info);
   142          return_ACPI_STATUS(status);

143 }

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#2 lore raw
Quoted 4 lines · Lv

Hi, Dan

Thanks for the report.
I'll double check it with C=2 enabled build.

It's a Smatch warning.

regards,
dan carpenter

#3 lore raw

PATCH 1/1 ACPICA: Namespace: Fix wrong cleanup code in acpi_ns_execute_table()

In reply to Dan Carpenter

An error was captured by the static checker. This patch fixes the issue. introduced in:
Subject: ACPICA: ACPI 2.0, Interpreter: Fix MLC issues by switching to
new TermList grammar for table loading

---
 drivers/acpi/acpica/nsparse.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)
drivers/acpi/acpica/nsparse.c +5 -3
index 2452bf3..72893a9 100644
@@ -135,10 +135,12 @@ acpi_ns_execute_table(u32 table_index, struct acpi_namespace_node *start_node)
 	(void)acpi_ut_acquire_mutex(ACPI_MTX_NAMESPACE);
 
 cleanup:
-	acpi_ut_remove_reference(method_obj);
-	ACPI_FREE(info->full_pathname);
-	info->full_pathname = NULL;
+	if (info) {
+		ACPI_FREE(info->full_pathname);
+		info->full_pathname = NULL;
+	}
 	ACPI_FREE(info);
+	acpi_ut_remove_reference(method_obj);
 	return_ACPI_STATUS(status);
 }
 
Signature · 1 line
1.7.10