Skip to contentSkip to conversation

ACPICA: Tables: Cleanup RSDP signature codes.

DISCUSSION Dan Carpenter 2 participants 3 messages Sep 26, 2013 – Oct 25, 2013 ACPI

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

Hello Lv Zheng,

I'm sorry to bother you about this. This code has been this way for years, but your recent formatting cleanups made it into something Smatch could understand and complain about. Hopefully, you could take a look?

The patch cacba8657351: "ACPICA: Tables: Cleanup RSDP signature codes." from Sep 23, 2013, leads to the following static checker warning: "drivers/acpi/acpica/tbprint.c:141 acpi_tb_print_table_header()

	 error: strncmp() '((((header->signature))))' too small (4 vs 8)"

drivers/acpi/acpica/tbprint.c
138 ACPI_INFO((AE_INFO, "%4.4s %p %05X",
139 header->signature, ACPI_CAST_PTR(void, address),
140 header->length));

   141          } else if (ACPI_VALIDATE_RSDP_SIG(header->signature)) {
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The RSDP_SIG is an 8 character signature but the header->signature buffer only has 4 characters so the signatures never match. What's the deal with that?

142
143 /* RSDP has no common fields */
144
145 ACPI_MEMCPY(local_header.oem_id,
146 ACPI_CAST_PTR(struct acpi_table_rsdp,
147 header)->oem_id, ACPI_OEM_ID_SIZE);

regards,
dan carpenter

author
#1 lore raw

Does anyone have an idea about this? The code is obviously wrong but the fix is unclear. Perhaps we should just note the bug in a comment?

regards,
dan carpenter

Quoted 31 lines · Dan Carpenter

Hello Lv Zheng,

I'm sorry to bother you about this. This code has been this way for years, but your recent formatting cleanups made it into something Smatch could understand and complain about. Hopefully, you could take a look?

The patch cacba8657351: "ACPICA: Tables: Cleanup RSDP signature codes." from Sep 23, 2013, leads to the following static checker warning: "drivers/acpi/acpica/tbprint.c:141 acpi_tb_print_table_header()

	 error: strncmp() '((((header->signature))))' too small (4 vs 8)"

drivers/acpi/acpica/tbprint.c
138 ACPI_INFO((AE_INFO, "%4.4s %p %05X",
139 header->signature, ACPI_CAST_PTR(void, address),

   140                             header->length));
   141          } else if (ACPI_VALIDATE_RSDP_SIG(header->signature)) {
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The RSDP_SIG is an 8 character signature but the header->signature buffer only has 4 characters so the signatures never match. What's the deal with that?

   142  
   143                  /* RSDP has no common fields */
   144  
   145                  ACPI_MEMCPY(local_header.oem_id,
   146                              ACPI_CAST_PTR(struct acpi_table_rsdp,
   147                                            header)->oem_id, ACPI_OEM_ID_SIZE);

regards,
dan carpenter

#2 lore raw
Quoted 2 lines · Dan Carpenter

Does anyone have an idea about this? The code is obviously wrong but the fix is unclear. Perhaps we should just note the bug in a comment?

The code is confusing but did things right.
The RSDP table header is different with other tables, its signature is actually 8 bytes. We passed the table address to this print function using the common table header pointer that caused the warning.

Perhaps we should change the prototype of acpi_tb_print_table_header to:
void
acpi_tb_print_table_header(acpi_physical_address address, void *header) and then cast it to appropriate type after we found which type of table it is really is. I suppose this will eliminate the warning?

Thanks,
Aaron

Quoted 40 lines

regards,
dan carpenter

Quoted 29 lines · Dan Carpenter

Hello Lv Zheng,

I'm sorry to bother you about this. This code has been this way for years, but your recent formatting cleanups made it into something Smatch could understand and complain about. Hopefully, you could take a look?

The patch cacba8657351: "ACPICA: Tables: Cleanup RSDP signature codes." from Sep 23, 2013, leads to the following static checker warning: "drivers/acpi/acpica/tbprint.c:141 acpi_tb_print_table_header()

	 error: strncmp() '((((header->signature))))' too small (4 vs 8)"

drivers/acpi/acpica/tbprint.c
138 ACPI_INFO((AE_INFO, "%4.4s %p %05X",
139 header->signature, ACPI_CAST_PTR(void, address),

   140                             header->length));
   141          } else if (ACPI_VALIDATE_RSDP_SIG(header->signature)) {
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The RSDP_SIG is an 8 character signature but the header->signature buffer only has 4 characters so the signatures never match. What's the deal with that?

   142  
   143                  /* RSDP has no common fields */
   144  
   145                  ACPI_MEMCPY(local_header.oem_id,
   146                              ACPI_CAST_PTR(struct acpi_table_rsdp,
   147                                            header)->oem_id, ACPI_OEM_ID_SIZE);
Quoted 2 lines

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