On Wed, 19 Dec 2018 10:20:36 +0100 Philippe Mathieu-Daudé <email address hidden> wrote:
> Le mer. 19 déc. 2018 10:16, Igor Mammedov <email address hidden> a écrit : > > > On Tue, 18 Dec 2018 18:51:20 +0100 > > Philippe Mathieu-Daudé <email address hidden> wrote: > > > > > GCC 8 added a -Wstringop-truncation warning: > > > > > > The -Wstringop-truncation warning added in GCC 8.0 via r254630 for > > > bug 81117 is specifically intended to highlight likely unintended > > > uses of the strncpy function that truncate the terminating NUL > > > character from the source string. > > > > > > This new warning leads to compilation failures: > > > > > > CC hw/acpi/core.o > > > In function 'acpi_table_install', inlined from 'acpi_table_add' at > > qemu/hw/acpi/core.c:296:5: > > > qemu/hw/acpi/core.c:184:9: error: 'strncpy' specified bound 4 equals > > destination size [-Werror=stringop-truncation] > > > strncpy(ext_hdr->sig, hdrs->sig, sizeof ext_hdr->sig); > > > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > make: *** [qemu/rules.mak:69: hw/acpi/core.o] Error 1 > > > > > > Use the QEMU_NONSTRING attribute, since ACPI tables don't require the > > > strings to be NUL-terminated. > > > > > > Suggested-by: Michael S. Tsirkin <email address hidden> > > > Signed-off-by: Philippe Mathieu-Daudé <email address hidden> > > > --- > > > hw/acpi/core.c | 8 ++++---- > > > include/hw/acpi/acpi-defs.h | 8 ++++---- > > > 2 files changed, 8 insertions(+), 8 deletions(-) > > > > > > diff --git a/hw/acpi/core.c b/hw/acpi/core.c > > > index aafdc61648..f60f750c3d 100644 > > > --- a/hw/acpi/core.c > > > +++ b/hw/acpi/core.c > > > @@ -35,14 +35,14 @@ > > > struct acpi_table_header { > > > uint16_t _length; /* our length, not actual part of the hdr > > */ > > > /* allows easier parsing for fw_cfg > > clients */ > > > - char sig[4]; /* ACPI signature (4 ASCII characters) */ > > > + char sig[4] QEMU_NONSTRING; /* ACPI signature (4 ASCII characters) > > */ > > > uint32_t length; /* Length of table, in bytes, including > > header */ > > > uint8_t revision; /* ACPI Specification minor version # */ > > > uint8_t checksum; /* To make sum of entire table == 0 */ > > > - char oem_id[6]; /* OEM identification */ > > > - char oem_table_id[8]; /* OEM table identification */ > > > + char oem_id[6] QEMU_NONSTRING; /* OEM identification */ > > > + char oem_table_id[8] QEMU_NONSTRING; /* OEM table identification */ > > > uint32_t oem_revision; /* OEM revision number */ > > > - char asl_compiler_id[4]; /* ASL compiler vendor ID */ > > > + char asl_compiler_id[4] QEMU_NONSTRING; /* ASL compiler vendor ID */ > > > uint32_t asl_compiler_revision; /* ASL compiler revision number */ > > > } QEMU_PACKED; > > > > > > diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h > > > index af8e023968..3bf0bec8ba 100644 > > > --- a/include/hw/acpi/acpi-defs.h > > > +++ b/include/hw/acpi/acpi-defs.h > > > @@ -43,7 +43,7 @@ enum { > > > struct AcpiRsdpDescriptor { /* Root System Descriptor Pointer */ > > > uint64_t signature; /* ACPI signature, contains "RSD > > PTR " */ > > > uint8_t checksum; /* To make sum of struct == 0 */ > > > - uint8_t oem_id [6]; /* OEM identification */ > > > + uint8_t oem_id [6] QEMU_NONSTRING; /* OEM identification */ > > > uint8_t revision; /* Must be 0 for 1.0, 2 for 2.0 */ > > > uint32_t rsdt_physical_address; /* 32-bit physical address of RSDT > > */ > > > uint32_t length; /* XSDT Length in bytes including > > hdr */ > > > > you'll need to rebase this on top the latest Michael's pull request. > > [PULL v2 25/30] hw: arm: Carry RSDP specific data through AcpiRsdpData > > [PULL v2 29/30] hw: acpi: Remove AcpiRsdpDescriptor and fix tests > > > > OK. Can I add your Ack-by then? pls note that new AcpiRsdpData has oem_id field that needs the same treatment
with rebase Reviewed-by: Igor Mammedov <email address hidden>
> > > @@ -62,10 +62,10 @@ typedef struct AcpiRsdpDescriptor AcpiRsdpDescriptor; > > > uint32_t length; /* Length of table, in bytes, > > including header */ \ > > > uint8_t revision; /* ACPI Specification minor > > version # */ \ > > > uint8_t checksum; /* To make sum of entire table == > > 0 */ \ > > > - uint8_t oem_id [6]; /* OEM identification */ \ > > > - uint8_t oem_table_id [8]; /* OEM table identification */ \ > > > + uint8_t oem_id [6] QEMU_NONSTRING; /* OEM identification */ \ > > > + uint8_t oem_table_id [8] QEMU_NONSTRING; /* OEM table > > identification */ \ > > > uint32_t oem_revision; /* OEM revision number */ \ > > > - uint8_t asl_compiler_id [4]; /* ASL compiler vendor ID */ \ > > > + uint8_t asl_compiler_id [4] QEMU_NONSTRING; /* ASL compiler vendor > > ID */ \ > > > uint32_t asl_compiler_revision; /* ASL compiler revision number */ > > > > > > > > > >
On Wed, 19 Dec 2018 10:20:36 +0100
Philippe Mathieu-Daudé <email address hidden> wrote:
> Le mer. 19 déc. 2018 10:16, Igor Mammedov <email address hidden> a écrit : truncation warning: truncation warning added in GCC 8.0 via r254630 for install' , inlined from 'acpi_table_add' at acpi/core. c:296:5: acpi/core. c:184:9: error: 'strncpy' specified bound 4 equals stringop- truncation] ext_hdr- >sig, hdrs->sig, sizeof ext_hdr->sig); ~~~~~~~ ~~~~~~~ ~~~~~~~ ~~~~~~~ ~~~~~~~ ~~~~~~~ ~~~~ hw/acpi/ acpi-defs. h | 8 ++++---- .f60f750c3d 100644 revision; /* ASL compiler revision number */ hw/acpi/ acpi-defs. h b/include/ hw/acpi/ acpi-defs. h .3bf0bec8ba 100644 hw/acpi/ acpi-defs. h hw/acpi/ acpi-defs. h address; /* 32-bit physical address of RSDT
>
> > On Tue, 18 Dec 2018 18:51:20 +0100
> > Philippe Mathieu-Daudé <email address hidden> wrote:
> >
> > > GCC 8 added a -Wstringop-
> > >
> > > The -Wstringop-
> > > bug 81117 is specifically intended to highlight likely unintended
> > > uses of the strncpy function that truncate the terminating NUL
> > > character from the source string.
> > >
> > > This new warning leads to compilation failures:
> > >
> > > CC hw/acpi/core.o
> > > In function 'acpi_table_
> > qemu/hw/
> > > qemu/hw/
> > destination size [-Werror=
> > > strncpy(
> > > ^~~~~~~
> > > make: *** [qemu/rules.mak:69: hw/acpi/core.o] Error 1
> > >
> > > Use the QEMU_NONSTRING attribute, since ACPI tables don't require the
> > > strings to be NUL-terminated.
> > >
> > > Suggested-by: Michael S. Tsirkin <email address hidden>
> > > Signed-off-by: Philippe Mathieu-Daudé <email address hidden>
> > > ---
> > > hw/acpi/core.c | 8 ++++----
> > > include/
> > > 2 files changed, 8 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> > > index aafdc61648.
> > > --- a/hw/acpi/core.c
> > > +++ b/hw/acpi/core.c
> > > @@ -35,14 +35,14 @@
> > > struct acpi_table_header {
> > > uint16_t _length; /* our length, not actual part of the hdr
> > */
> > > /* allows easier parsing for fw_cfg
> > clients */
> > > - char sig[4]; /* ACPI signature (4 ASCII characters) */
> > > + char sig[4] QEMU_NONSTRING; /* ACPI signature (4 ASCII characters)
> > */
> > > uint32_t length; /* Length of table, in bytes, including
> > header */
> > > uint8_t revision; /* ACPI Specification minor version # */
> > > uint8_t checksum; /* To make sum of entire table == 0 */
> > > - char oem_id[6]; /* OEM identification */
> > > - char oem_table_id[8]; /* OEM table identification */
> > > + char oem_id[6] QEMU_NONSTRING; /* OEM identification */
> > > + char oem_table_id[8] QEMU_NONSTRING; /* OEM table identification */
> > > uint32_t oem_revision; /* OEM revision number */
> > > - char asl_compiler_id[4]; /* ASL compiler vendor ID */
> > > + char asl_compiler_id[4] QEMU_NONSTRING; /* ASL compiler vendor ID */
> > > uint32_t asl_compiler_
> > > } QEMU_PACKED;
> > >
> > > diff --git a/include/
> > > index af8e023968.
> > > --- a/include/
> > > +++ b/include/
> > > @@ -43,7 +43,7 @@ enum {
> > > struct AcpiRsdpDescriptor { /* Root System Descriptor Pointer */
> > > uint64_t signature; /* ACPI signature, contains "RSD
> > PTR " */
> > > uint8_t checksum; /* To make sum of struct == 0 */
> > > - uint8_t oem_id [6]; /* OEM identification */
> > > + uint8_t oem_id [6] QEMU_NONSTRING; /* OEM identification */
> > > uint8_t revision; /* Must be 0 for 1.0, 2 for 2.0 */
> > > uint32_t rsdt_physical_
> > */
> > > uint32_t length; /* XSDT Length in bytes including
> > hdr */
> >
> > you'll need to rebase this on top the latest Michael's pull request.
> > [PULL v2 25/30] hw: arm: Carry RSDP specific data through AcpiRsdpData
> > [PULL v2 29/30] hw: acpi: Remove AcpiRsdpDescriptor and fix tests
> >
>
> OK. Can I add your Ack-by then?
pls note that new AcpiRsdpData has oem_id field that needs the same treatment
with rebase
Reviewed-by: Igor Mammedov <email address hidden>
> revision; /* ASL compiler revision number */
> > @@ -62,10 +62,10 @@ typedef struct AcpiRsdpDescriptor AcpiRsdpDescriptor;
> > > uint32_t length; /* Length of table, in bytes,
> > including header */ \
> > > uint8_t revision; /* ACPI Specification minor
> > version # */ \
> > > uint8_t checksum; /* To make sum of entire table ==
> > 0 */ \
> > > - uint8_t oem_id [6]; /* OEM identification */ \
> > > - uint8_t oem_table_id [8]; /* OEM table identification */ \
> > > + uint8_t oem_id [6] QEMU_NONSTRING; /* OEM identification */ \
> > > + uint8_t oem_table_id [8] QEMU_NONSTRING; /* OEM table
> > identification */ \
> > > uint32_t oem_revision; /* OEM revision number */ \
> > > - uint8_t asl_compiler_id [4]; /* ASL compiler vendor ID */ \
> > > + uint8_t asl_compiler_id [4] QEMU_NONSTRING; /* ASL compiler vendor
> > ID */ \
> > > uint32_t asl_compiler_
> > >
> > >
> >
> >