From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37353) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHpze-0004ob-0S for qemu-devel@nongnu.org; Sat, 09 Jan 2016 04:39:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aHpza-0005i5-QD for qemu-devel@nongnu.org; Sat, 09 Jan 2016 04:39:29 -0500 Received: from [59.151.112.132] (port=34331 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aHpza-0005gG-CK for qemu-devel@nongnu.org; Sat, 09 Jan 2016 04:39:26 -0500 References: <1452242274-8345-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1452242274-8345-2-git-send-email-caoj.fnst@cn.fujitsu.com> <568FF368.7080308@redhat.com> From: Cao jin Message-ID: <5690D5F5.2010508@cn.fujitsu.com> Date: Sat, 9 Jan 2016 17:42:13 +0800 MIME-Version: 1.0 In-Reply-To: <568FF368.7080308@redhat.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v4 1/5] Use qemu_strtoul instead of strtol List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: qemu-devel@nongnu.org, stefano.stabellini@eu.citrix.com On 01/09/2016 01:35 AM, Eric Blake wrote: > On 01/08/2016 01:37 AM, Cao jin wrote: >> strtol() don`t guarantee errno to be ERANGE on overflow. > > I stand slightly corrected: C99 requires ERANGE on overflow, but not > EINVAL; it is POSIX that adds EINVAL, but does not properly require it. > At any rate, my main point was that errno is not always properly set by > all strtol implementations, and furthermore that you can't rely on it > being set to a sane value if you didn't pre-set it to 0. > Got you:) >> This wrapper returns either -EINVAL or the errno set by strtol() >> function (e.g -ERANGE). > > The subject line doesn't start with a topic. Maybe a better commit > message would be: > > xen: Use qemu_strtoul instead of strtol > > No need to roll our own (with slightly incorrect handling of errno), > when we can use the common version. > ok, looks good. >> >> Signed-off-by: Cao jin >> --- >> hw/xen/xen-host-pci-device.c | 11 +++-------- >> 1 file changed, 3 insertions(+), 8 deletions(-) > >> buf[rc] = 0; >> - value = strtol(buf, &endptr, base); >> - if (endptr == buf || *endptr != '\n') { >> - rc = -1; >> - } else if ((value == LONG_MIN || value == LONG_MAX) && errno == ERANGE) { >> - rc = -errno; >> - } else { >> - rc = 0; >> + rc = qemu_strtoul(buf, &endptr, base, &value); > > Why did you switch from strtol() to qemu_strtoul()? Was signed parsing > incorrect, and unsigned parsing a bug fix? If so, please mention it in > the commit message as intentional. Otherwise, use qemu_strtol() (and > adjust the commit message accordingly). > Yes, I should mention it, sorry for forgetting to explain it. Because what it want to read from host are values in pci device config space, which are non-negative, like "vendor ID", "device ID", "class code", etc. so unsigned parsing seems more suitable in this case. we can tell it also from the local variable "unsigned long value;" -- Yours Sincerely, Cao jin