On Tue, 18 Dec 2018 12:03:31 +0100
Philippe Mathieu-Daudé <phi...@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lur...@redhat.com>
>
> 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
>
> The ACPI tables don't require the strings to be NUL-terminated,
> therefore strncpy is the right function to use here.
>
> We could add a #pragma GCC diagnostic ignored "-Wstringop-truncation"
> around, disable the warning globally using -Wno-stringop-truncation,
> but since QEMU provides the strpadcpy() which does the same purpose,
> simply use it to avoid the annoying warning.
>
> Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>
> [PMD: reword commit subject and description]
> Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
Reviewed-by: Igor Mammedov <imamm...@redhat.com>
> ---
> hw/acpi/aml-build.c | 6 ++++--
> hw/acpi/core.c | 13 +++++++------
> 2 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 1e43cd736d..397833462a 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -24,6 +24,7 @@
> #include "hw/acpi/aml-build.h"
> #include "qemu/bswap.h"
> #include "qemu/bitops.h"
> +#include "qemu/cutils.h"
> #include "sysemu/numa.h"
>
> static GArray *build_alloc_array(void)
> @@ -1532,13 +1533,14 @@ build_header(BIOSLinker *linker, GArray *table_data,
> h->revision = rev;
>
> if (oem_id) {
> - strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);
> + strpadcpy((char *)h->oem_id, sizeof h->oem_id, oem_id, '\0');
> } else {
> memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
> }
>
> if (oem_table_id) {
> - strncpy((char *)h->oem_table_id, oem_table_id,
> sizeof(h->oem_table_id));
> + strpadcpy((char *)h->oem_table_id, sizeof(h->oem_table_id),
> + oem_table_id, '\0');
> } else {
> memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);
> memcpy(h->oem_table_id + 4, sig, 4);
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index aafdc61648..6e8f4e5713 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -31,6 +31,7 @@
> #include "qapi/qapi-visit-misc.h"
> #include "qemu/error-report.h"
> #include "qemu/option.h"
> +#include "qemu/cutils.h"
>
> struct acpi_table_header {
> uint16_t _length; /* our length, not actual part of the hdr */
> @@ -181,7 +182,7 @@ static void acpi_table_install(const char unsigned *blob,
> size_t bloblen,
> ext_hdr->_length = cpu_to_le16(acpi_payload_size);
>
> if (hdrs->has_sig) {
> - strncpy(ext_hdr->sig, hdrs->sig, sizeof ext_hdr->sig);
> + strpadcpy(ext_hdr->sig, sizeof ext_hdr->sig, hdrs->sig, '\0');
> ++changed_fields;
> }
>
> @@ -200,12 +201,12 @@ static void acpi_table_install(const char unsigned
> *blob, size_t bloblen,
> ext_hdr->checksum = 0;
>
> if (hdrs->has_oem_id) {
> - strncpy(ext_hdr->oem_id, hdrs->oem_id, sizeof ext_hdr->oem_id);
> + strpadcpy(ext_hdr->oem_id, sizeof ext_hdr->oem_id, hdrs->oem_id,
> '\0');
> ++changed_fields;
> }
> if (hdrs->has_oem_table_id) {
> - strncpy(ext_hdr->oem_table_id, hdrs->oem_table_id,
> - sizeof ext_hdr->oem_table_id);
> + strpadcpy(ext_hdr->oem_table_id, sizeof ext_hdr->oem_table_id,
> + hdrs->oem_table_id, '\0');
> ++changed_fields;
> }
> if (hdrs->has_oem_rev) {
> @@ -213,8 +214,8 @@ static void acpi_table_install(const char unsigned *blob,
> size_t bloblen,
> ++changed_fields;
> }
> if (hdrs->has_asl_compiler_id) {
> - strncpy(ext_hdr->asl_compiler_id, hdrs->asl_compiler_id,
> - sizeof ext_hdr->asl_compiler_id);
> + strpadcpy(ext_hdr->asl_compiler_id, sizeof ext_hdr->asl_compiler_id,
> + hdrs->asl_compiler_id, '\0');
> ++changed_fields;
> }
> if (hdrs->has_asl_compiler_rev) {
