* Re: [Qemu-devel] sheepdog build warning
2018-12-18 4:16 ` Michael S. Tsirkin
@ 2018-12-18 10:14 ` Philippe Mathieu-Daudé
2018-12-18 10:35 ` Daniel P. Berrangé
1 sibling, 0 replies; 4+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-12-18 10:14 UTC (permalink / raw)
To: Michael S. Tsirkin, qemu-devel
Cc: Kevin Wolf, peter.maydell, qemu-block, meyering, Jeff Cody,
Liu Yuan, MORITA Kazutaka
Hi Michael,
On 12/18/18 5:16 AM, Michael S. Tsirkin wrote:
> On Mon, Dec 17, 2018 at 11:03:11PM -0500, Michael S. Tsirkin wrote:
>> mingw32 build on fedora fails with this warning:
>>
>> /scm/qemu/block/sheepdog.c: In function 'find_vdi_name':
>> /scm/qemu/block/sheepdog.c:1239:5: error: 'strncpy' specified bound 256 equals destination size [-Werror=stringop-truncation]
>> strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN);
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> cc1: all warnings being treated as errors
>> make: *** [/scm/qemu/rules.mak:69: block/sheepdog.o] Error 1
>> make: *** Waiting for unfinished jobs....
>>
>>
>> Reading the code one sees it's working as intended:
>>
>>
>> static int find_vdi_name(BDRVSheepdogState *s, const char *filename,
>> uint32_t snapid, const char *tag, uint32_t *vid,
>> bool lock, Error **errp)
>> {
>> int ret, fd;
>> SheepdogVdiReq hdr;
>> SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr;
>> unsigned int wlen, rlen = 0;
>> char buf[SD_MAX_VDI_LEN + SD_MAX_VDI_TAG_LEN];
>>
>> fd = connect_to_sdog(s, errp);
>> if (fd < 0) {
>> return fd;
>> }
>>
>> /* This pair of strncpy calls ensures that the buffer is zero-filled,
>> * which is desirable since we'll soon be sending those bytes, and
>> * don't want the send_req to read uninitialized data.
>> */
>> strncpy(buf, filename, SD_MAX_VDI_LEN);
>> strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN);
>>
>>
>> .....
>>
>> }
>>
>>
>>
>> so this seems to be the case of GCC developers deciding that
>> strncpy is simply a bad API and a correct use of it should be
>> warned against.
>>
>> I propose either
>>
>> 1. simply adding
>>
>> #pragma GCC diagnostic ignored "-Wstringop-truncation"
>>
>> in osdep.
>>
>> 2. adding an inline wrapper with said pragma in there.
>>
>> 3. -Wno-stringop-truncation is the makefile
>>
>> Thoughts?
>>
>> --
>
>
> So here's approach 2. However I note that a newer gcc 8.2.1
> does not give this warning. Maybe detect at configure time
> and suppress (option 3)?
I prepared patches yesterday to fix this issue, I'll send to the list
soon. I used another approach, replacing strncpy() by strpadcpy(pad=0).
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 3bf48bcdec..64d8258529 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -134,6 +134,22 @@ extern int daemon(int, int);
> #define assert(x) g_assert(x)
> #endif
>
> +/*
> + * GCC 8.0 declared war on strncpy. Admittedly it's a tricky interface
> + * with unintuitive semantics, but we use it widely.
> + */
> +#if defined(__GNUC__) && __GNUC__ >= 8
> +static inline void qemu_strncpy(char *to, const char *from, int n)
> +{
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
> + strncpy(to, from, n);
> +#pragma GCC diagnostic pop
> +}
> +
> +#define strncpy qemu_strncpy
> +#endif
> +
> /*
> * According to waitpid man page:
> * WCOREDUMP
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] sheepdog build warning
2018-12-18 4:16 ` Michael S. Tsirkin
2018-12-18 10:14 ` Philippe Mathieu-Daudé
@ 2018-12-18 10:35 ` Daniel P. Berrangé
1 sibling, 0 replies; 4+ messages in thread
From: Daniel P. Berrangé @ 2018-12-18 10:35 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, Kevin Wolf, peter.maydell, qemu-block, meyering,
Jeff Cody, Liu Yuan, MORITA Kazutaka
On Mon, Dec 17, 2018 at 11:16:05PM -0500, Michael S. Tsirkin wrote:
> On Mon, Dec 17, 2018 at 11:03:11PM -0500, Michael S. Tsirkin wrote:
> > mingw32 build on fedora fails with this warning:
> >
> > /scm/qemu/block/sheepdog.c: In function 'find_vdi_name':
> > /scm/qemu/block/sheepdog.c:1239:5: error: 'strncpy' specified bound 256 equals destination size [-Werror=stringop-truncation]
> > strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN);
> > ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > cc1: all warnings being treated as errors
> > make: *** [/scm/qemu/rules.mak:69: block/sheepdog.o] Error 1
> > make: *** Waiting for unfinished jobs....
> >
> >
> > Reading the code one sees it's working as intended:
> >
> >
> > static int find_vdi_name(BDRVSheepdogState *s, const char *filename,
> > uint32_t snapid, const char *tag, uint32_t *vid,
> > bool lock, Error **errp)
> > {
> > int ret, fd;
> > SheepdogVdiReq hdr;
> > SheepdogVdiRsp *rsp = (SheepdogVdiRsp *)&hdr;
> > unsigned int wlen, rlen = 0;
> > char buf[SD_MAX_VDI_LEN + SD_MAX_VDI_TAG_LEN];
> >
> > fd = connect_to_sdog(s, errp);
> > if (fd < 0) {
> > return fd;
> > }
> >
> > /* This pair of strncpy calls ensures that the buffer is zero-filled,
> > * which is desirable since we'll soon be sending those bytes, and
> > * don't want the send_req to read uninitialized data.
> > */
> > strncpy(buf, filename, SD_MAX_VDI_LEN);
> > strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN);
> >
> >
> > .....
> >
> > }
> >
> >
> >
> > so this seems to be the case of GCC developers deciding that
> > strncpy is simply a bad API and a correct use of it should be
> > warned against.
> >
> > I propose either
> >
> > 1. simply adding
> >
> > #pragma GCC diagnostic ignored "-Wstringop-truncation"
> >
> > in osdep.
> >
> > 2. adding an inline wrapper with said pragma in there.
> >
> > 3. -Wno-stringop-truncation is the makefile
> >
> > Thoughts?
> >
> > --
>
>
> So here's approach 2. However I note that a newer gcc 8.2.1
> does not give this warning. Maybe detect at configure time
> and suppress (option 3)?
I'm not
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 3bf48bcdec..64d8258529 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -134,6 +134,22 @@ extern int daemon(int, int);
> #define assert(x) g_assert(x)
> #endif
>
> +/*
> + * GCC 8.0 declared war on strncpy. Admittedly it's a tricky interface
> + * with unintuitive semantics, but we use it widely.
> + */
> +#if defined(__GNUC__) && __GNUC__ >= 8
> +static inline void qemu_strncpy(char *to, const char *from, int n)
> +{
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Wstringop-truncation"
> + strncpy(to, from, n);
> +#pragma GCC diagnostic pop
> +}
> +
> +#define strncpy qemu_strncpy
> +#endif
I don't think we should disable the warning for all uses of strncpy
as that defeats the protection this warning offers for bad usage.
IMHO, just push/pop the warning pragma inline in the sheepdog code
which triggers it.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 4+ messages in thread