* [PATCH] APEI: pull a signedness check ahead for Coverity's sake
@ 2016-06-08 11:37 Jan Beulich
2016-06-09 14:13 ` Andrew Cooper
2016-06-09 14:24 ` Julien Grall
0 siblings, 2 replies; 4+ messages in thread
From: Jan Beulich @ 2016-06-08 11:37 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1: Type: text/plain, Size: 670 bytes --]
On 64-bit architectures (which is all we care about right now in ACPI
code), the value coming from a __u32 field makes "len" positive anyway,
but since from an abstract pov the tool is right, let's just re-order
things.
Coverity ID: 1204965
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/acpi/apei/erst.c
+++ b/xen/drivers/acpi/apei/erst.c
@@ -672,9 +672,11 @@ static ssize_t __erst_read(u64 record_id
if (rcd_tmp->record_length > buflen)
return -ENOBUFS;
len = rcd_tmp->record_length;
+ if (len < 0)
+ return -ERANGE;
memcpy(record, rcd_tmp, len);
- return len >= 0 ? len : -ERANGE;
+ return len;
}
/*
[-- Attachment #2: CID1204965.patch --]
[-- Type: text/plain, Size: 723 bytes --]
APEI: pull a signedness check ahead for Coverity's sake
On 64-bit architectures (which is all we care about right now in ACPI
code), the value coming from a __u32 field makes "len" positive anyway,
but since from an abstract pov the tool is right, let's just re-order
things.
Coverity ID: 1204965
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/acpi/apei/erst.c
+++ b/xen/drivers/acpi/apei/erst.c
@@ -672,9 +672,11 @@ static ssize_t __erst_read(u64 record_id
if (rcd_tmp->record_length > buflen)
return -ENOBUFS;
len = rcd_tmp->record_length;
+ if (len < 0)
+ return -ERANGE;
memcpy(record, rcd_tmp, len);
- return len >= 0 ? len : -ERANGE;
+ return len;
}
/*
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] APEI: pull a signedness check ahead for Coverity's sake
2016-06-08 11:37 [PATCH] APEI: pull a signedness check ahead for Coverity's sake Jan Beulich
@ 2016-06-09 14:13 ` Andrew Cooper
2016-06-09 14:24 ` Julien Grall
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Cooper @ 2016-06-09 14:13 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 08/06/16 12:37, Jan Beulich wrote:
> On 64-bit architectures (which is all we care about right now in ACPI
> code), the value coming from a __u32 field makes "len" positive anyway,
> but since from an abstract pov the tool is right, let's just re-order
> things.
>
> Coverity ID: 1204965
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] APEI: pull a signedness check ahead for Coverity's sake
2016-06-08 11:37 [PATCH] APEI: pull a signedness check ahead for Coverity's sake Jan Beulich
2016-06-09 14:13 ` Andrew Cooper
@ 2016-06-09 14:24 ` Julien Grall
2016-06-09 15:08 ` Jan Beulich
1 sibling, 1 reply; 4+ messages in thread
From: Julien Grall @ 2016-06-09 14:24 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Hi Jan,
On 08/06/16 12:37, Jan Beulich wrote:
> On 64-bit architectures (which is all we care about right now in ACPI
> code), the value coming from a __u32 field makes "len" positive anyway,
> but since from an abstract pov the tool is right, let's just re-order
> things.
All the usage of len are unsigned, so why do we want to keep len signed?
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] APEI: pull a signedness check ahead for Coverity's sake
2016-06-09 14:24 ` Julien Grall
@ 2016-06-09 15:08 ` Jan Beulich
0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2016-06-09 15:08 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel
>>> On 09.06.16 at 16:24, <julien.grall@arm.com> wrote:
> On 08/06/16 12:37, Jan Beulich wrote:
>> On 64-bit architectures (which is all we care about right now in ACPI
>> code), the value coming from a __u32 field makes "len" positive anyway,
>> but since from an abstract pov the tool is right, let's just re-order
>> things.
>
> All the usage of len are unsigned, so why do we want to keep len signed?
Because it serves as the return value of the function, and the
function's return type is signed. Also note how the change makes
the code correct even for 32-bit architectures, even if we don't
care about such in ACPI code right now.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-06-09 15:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-08 11:37 [PATCH] APEI: pull a signedness check ahead for Coverity's sake Jan Beulich
2016-06-09 14:13 ` Andrew Cooper
2016-06-09 14:24 ` Julien Grall
2016-06-09 15:08 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).