Re: [Qemu-devel] [PATCH v3 3/5] hw/acpi: Use QEMU_NONSTRING for non NUL-terminated arrays

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On Thu, Dec 20, 2018 at 4:18 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Wed, 19 Dec 2018 14:00:37 +0100
> Andrew Jones <drjones@redhat.com> wrote:
>
> > On Wed, Dec 19, 2018 at 01:43:40PM +0100, Philippe Mathieu-Daudé wrote:
> > > Hi Drew,
> > >
> > > On 12/19/18 11:10 AM, Andrew Jones wrote:
> > > > On Tue, Dec 18, 2018 at 06:51:20PM +0100, Philippe Mathieu-Daudé 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.
> > > >
> > > > Aren't we always starting with zero-initialized structures in ACPI code?
> > > > If so, then we should be able to change the strncpy's to memcpy's.
> > >
> > > The first call zero-initializes, but then we call realloc():
> > >
> > >     /* We won't fail from here on. Initialize / extend the globals. */
> > >     if (acpi_tables == NULL) {
> > >         acpi_tables_len = sizeof(uint16_t);
> > >         acpi_tables = g_malloc0(acpi_tables_len);
> > >     }
> > >
> > >     acpi_tables = g_realloc(acpi_tables, acpi_tables_len +
> > >                                          ACPI_TABLE_PFX_SIZE +
> > >                                          sizeof dfl_hdr + body_size);
> > >
> > >     ext_hdr = (struct acpi_table_header *)(acpi_tables +
> > >                                            acpi_tables_len);
> > >
> > > So memcpy() isn't enough.
> >
> > Ah, thanks.
> >
> > >
> > > I can resend the previous patch which uses strpadcpy() if you prefer,
> > > Igor already reviewed it:
> > >
> > > https://lists.gnu.org/archive/html/qemu-devel/2018-12/msg04406.html
> > >
> >
> > I do like strpadcpy() better, but I'm not going to lose sleep about
> > this either way it goes.
> I'm ok with both ways, but v2 consensus was to use QEMU_NONSTRING if I got it right

Yes, MST recommended it because this attribute is clever than strpadcpy().