* re: ACPICA: Tables: Cleanup RSDP signature codes.
@ 2013-09-26 22:35 Dan Carpenter
2013-10-17 12:39 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2013-09-26 22:35 UTC (permalink / raw)
To: lv.zheng; +Cc: linux-acpi
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: ACPICA: Tables: Cleanup RSDP signature codes.
2013-09-26 22:35 ACPICA: Tables: Cleanup RSDP signature codes Dan Carpenter
@ 2013-10-17 12:39 ` Dan Carpenter
2013-10-25 5:49 ` Aaron Lu
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2013-10-17 12:39 UTC (permalink / raw)
To: lv.zheng; +Cc: linux-acpi
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
On Fri, Sep 27, 2013 at 01:35:20AM +0300, Dan Carpenter wrote:
> 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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: ACPICA: Tables: Cleanup RSDP signature codes.
2013-10-17 12:39 ` Dan Carpenter
@ 2013-10-25 5:49 ` Aaron Lu
0 siblings, 0 replies; 3+ messages in thread
From: Aaron Lu @ 2013-10-25 5:49 UTC (permalink / raw)
To: Dan Carpenter; +Cc: lv.zheng, linux-acpi, Robert Moore
On Thu, Oct 17, 2013 at 03:39:42PM +0300, Dan Carpenter wrote:
> 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
>
> regards,
> dan carpenter
>
> On Fri, Sep 27, 2013 at 01:35:20AM +0300, Dan Carpenter wrote:
> > 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-10-25 5:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-26 22:35 ACPICA: Tables: Cleanup RSDP signature codes Dan Carpenter
2013-10-17 12:39 ` Dan Carpenter
2013-10-25 5:49 ` Aaron Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).