mbox series

[v4,0/5] Fix strncpy() warnings for GCC8 new -Wstringop-truncation

Message ID 20181228173356.15359-1-philmd@redhat.com
Headers show
Series Fix strncpy() warnings for GCC8 new -Wstringop-truncation | expand

Message

Philippe Mathieu-Daudé Dec. 28, 2018, 5:33 p.m. UTC
GCC 8 new warning prevents builds to success since quite some time.
First report on the mailing list is in July 2018:
https://lists.gnu.org/archive/html/qemu-devel/2018-07/msg03723.html

Since v3:
- patch 1: make sens of description (eblake)
- patch 2: append QEMU_NONSTRING instead of prepending it (mst)
- patch 3: rebased (imammedo), intented
- patch 4: replaced by Marc-André first attempt, improved doc
- patch 5: add assert() and NUL-terminate the buffer (mst)

Various intents has been sent to fix this:
- Incorrectly using g_strlcpy()
  https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03705.html
  https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg03706.html
- Using assert() and strpadcpy()
  https://lists.gnu.org/archive/html/qemu-devel/2018-11/msg03938.html
  This was the approch taken by the previous v2:
  https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04405.html
- Use #pragma GCC diagnostic ignored "-Wstringop-truncation"
  https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04261.html
- adding an inline wrapper with said pragma in there
  https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04261.html
- -Wno-stringop-truncation is the makefile
  https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04261.html
- Use the 'nonstring' attribute
  https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04493.html

This series add the QEMU_NONSTRING definition and use it.

Regards,

Phil.

Marc-André Lureau (1):
  migration: Fix stringop-truncation warning

Philippe Mathieu-Daudé (4):
  qemu/compiler: Define QEMU_NONSTRING
  block/sheepdog: Use QEMU_NONSTRING for non NUL-terminated arrays
  hw/acpi: Use QEMU_NONSTRING for non NUL-terminated arrays
  migration: Use strnlen() for fixed-size string

 block/sheepdog.c            |  2 +-
 hw/acpi/core.c              | 12 ++++++++----
 include/hw/acpi/acpi-defs.h | 13 ++++++++-----
 include/qemu/compiler.h     | 15 +++++++++++++++
 migration/global_state.c    | 14 +++++++++++++-
 5 files changed, 45 insertions(+), 11 deletions(-)

Comments

no-reply@patchew.org Jan. 2, 2019, 3:21 p.m. UTC | #1
Patchew URL: https://patchew.org/QEMU/20181228173356.15359-1-philmd@redhat.com/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20181228173356.15359-1-philmd@redhat.com
Subject: [Qemu-devel] [PATCH v4 0/5] Fix strncpy() warnings for GCC8 new -Wstringop-truncation

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
7afec15 migration: Use strnlen() for fixed-size string
3f0d878 migration: Fix stringop-truncation warning
f6b1b52 hw/acpi: Use QEMU_NONSTRING for non NUL-terminated arrays
16e5ce8 block/sheepdog: Use QEMU_NONSTRING for non NUL-terminated arrays
f4ac58a qemu/compiler: Define QEMU_NONSTRING

=== OUTPUT BEGIN ===
Checking PATCH 1/5: qemu/compiler: Define QEMU_NONSTRING...
WARNING: architecture specific defines should be avoided
#53: FILE: include/qemu/compiler.h:163:
+#if __has_attribute(nonstring)

total: 0 errors, 1 warnings, 21 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 2/5: block/sheepdog: Use QEMU_NONSTRING for non NUL-terminated arrays...
Checking PATCH 3/5: hw/acpi: Use QEMU_NONSTRING for non NUL-terminated arrays...
ERROR: space prohibited before open square bracket '['
#82: FILE: include/hw/acpi/acpi-defs.h:60:
+    uint8_t  oem_id [6] \

WARNING: Block comments use a leading /* on a separate line
#83: FILE: include/hw/acpi/acpi-defs.h:61:
+                 QEMU_NONSTRING;     /* OEM identification */ \

ERROR: space prohibited before open square bracket '['
#84: FILE: include/hw/acpi/acpi-defs.h:62:
+    uint8_t  oem_table_id [8] \

WARNING: Block comments use a leading /* on a separate line
#85: FILE: include/hw/acpi/acpi-defs.h:63:
+                 QEMU_NONSTRING;     /* OEM table identification */ \

ERROR: space prohibited before open square bracket '['
#88: FILE: include/hw/acpi/acpi-defs.h:65:
+    uint8_t  asl_compiler_id [4] \

WARNING: Block comments use a leading /* on a separate line
#89: FILE: include/hw/acpi/acpi-defs.h:66:
+                 QEMU_NONSTRING;     /* ASL compiler vendor ID */ \

total: 3 errors, 3 warnings, 48 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/5: migration: Fix stringop-truncation warning...
Checking PATCH 5/5: migration: Use strnlen() for fixed-size string...
WARNING: Block comments use a leading /* on a separate line
#49: FILE: migration/global_state.c:94:
+        /* This condition should never happen during migration, because

total: 0 errors, 1 warnings, 25 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20181228173356.15359-1-philmd@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Philippe Mathieu-Daudé Jan. 3, 2019, 8:40 a.m. UTC | #2
On 1/2/19 4:21 PM, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20181228173356.15359-1-philmd@redhat.com/
[...]> === OUTPUT BEGIN ===
> Checking PATCH 1/5: qemu/compiler: Define QEMU_NONSTRING...
> WARNING: architecture specific defines should be avoided
> #53: FILE: include/qemu/compiler.h:163:
> +#if __has_attribute(nonstring)

I assume this one is OK, this is consistent with the file.

> Checking PATCH 3/5: hw/acpi: Use QEMU_NONSTRING for non NUL-terminated arrays...
> ERROR: space prohibited before open square bracket '['
> #82: FILE: include/hw/acpi/acpi-defs.h:60:
> +    uint8_t  oem_id [6] \

I wanted to respect the maintainer previous indentation, but I'll fix.

> 
> WARNING: Block comments use a leading /* on a separate line
> #83: FILE: include/hw/acpi/acpi-defs.h:61:
> +                 QEMU_NONSTRING;     /* OEM identification */ \

Eh this is a multi-line macro... I don't think we can split this
comment. I will keep it that way.

> Checking PATCH 5/5: migration: Use strnlen() for fixed-size string...
> WARNING: Block comments use a leading /* on a separate line
> #49: FILE: migration/global_state.c:94:
> +        /* This condition should never happen during migration, because

Well, this is not a comment describing a function but within a code
block. I find it cleaner this way, but matter of taste, so I'll fix.

Regards,

Phil.