All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>,
	QEMU Developers <qemu-devel@nongnu.org>
Cc: John Snow <jsnow@redhat.com>
Subject: Re: [Qemu-devel] qtest protocol: should memset/read/write etc of a size of 0 bytes be permitted?
Date: Thu, 4 Aug 2016 14:39:12 -0600	[thread overview]
Message-ID: <57A3A7F0.3000604@redhat.com> (raw)
In-Reply-To: <CAFEAcA9Hr_xicYgob-1viu9+vOZ8kuFhuym10gcx5mHF=pC_0Q@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1571 bytes --]

On 08/04/2016 12:46 PM, Peter Maydell wrote:
> I've upgraded to a more recent version of clang, which now produces
> undefined-behaviour warnings for passing NULL pointers to some library
> functions. One of the things it has shown up is that some of the
> qtest tests ask for "memset" with size zero. In our current implementation
> this results in qtest.c calling g_malloc(0), which returns NULL, and

I never understood why glib made that choice on g_malloc(0). I would
much prefer it to ALWAYS return something, just as glibc malloc(0) does.

> then calling memset(NULL, chr, 0), which is UB.

Indeed, although I really wish POSIX could be loosened to say that the
source pointer is untouched if the length is 0 (I've debated about
filing a POSIX bug report to that effect, but have not done so yet), so
that the UB only happens when passing NULL with a non-zero size.

> 
> So should we:
> (1) declare the qtest protocol commands 'memset', 'read', 'write'
> etc which operate on a lump of guest memory of specified size to
> support size == 0 as meaning "do nothing"

My preference - even if we have to special case things to avoid UB at
the lower level, presenting well-defined behavior at the upper level is
easier to think about.

> (2) declare that size == 0 is not valid and make it return a failure
> code back down the qtest pipe (and fix the offending tests)

Doable, but not as fun to audit, and not my preference.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

  parent reply	other threads:[~2016-08-04 20:39 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-04 18:46 [Qemu-devel] qtest protocol: should memset/read/write etc of a size of 0 bytes be permitted? Peter Maydell
2016-08-04 18:49 ` John Snow
2016-08-04 20:11   ` Peter Maydell
2016-08-04 20:39 ` Eric Blake [this message]
2016-08-05  6:46   ` Markus Armbruster
2016-08-05  9:47     ` Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57A3A7F0.3000604@redhat.com \
    --to=eblake@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.