1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
|
4 * redundant conditions
1.
[qemu/hw/block/nvme.c:355]: (style) Redundant condition: sqid. 'A && (!A || B)' is equivalent to 'A || B'
if (!sqid || (sqid && !nvme_check_sqid(n, sqid))) {
2.
[qemu/hw/block/nvme.c:429]: (style) Redundant condition: cqid. 'A && (!A || B)' is equivalent to 'A || B'
if (!cqid || (cqid && !nvme_check_cqid(n, cqid))) {
3.
[qemu/hw/tpm/tpm_passthrough.c:157]: (style) Redundant condition: tpm_pt.tpm_op_canceled. 'A && (!A || B)' is equivalent to 'A || B'
if (!tpm_pt->tpm_op_canceled ||
(tpm_pt->tpm_op_canceled && errno != ECANCELED)) {
4.
[qemu/target-arm/translate-a64.c:5729]: (style) Redundant condition: size<3. 'A && (!A || B)' is equivalent to 'A || B'
if (size > 3
|| (size < 3 && is_q)
|| (size == 3 && !is_q)) {
On 12 June 2015 at 11:38, dcb <email address hidden> wrote:
> Public bug reported:
>
>
> 1.
>
> [qemu/hw/block/nvme.c:355]: (style) Redundant condition: sqid. 'A && (!A
> || B)' is equivalent to 'A || B'
>
> if (!sqid || (sqid && !nvme_check_sqid(n, sqid))) {
>
> 2.
>
> [qemu/hw/block/nvme.c:429]: (style) Redundant condition: cqid. 'A && (!A
> || B)' is equivalent to 'A || B'
>
> if (!cqid || (cqid && !nvme_check_cqid(n, cqid))) {
>
> 3.
>
> [qemu/hw/tpm/tpm_passthrough.c:157]: (style) Redundant condition:
> tpm_pt.tpm_op_canceled. 'A && (!A || B)' is equivalent to 'A || B'
>
> if (!tpm_pt->tpm_op_canceled ||
> (tpm_pt->tpm_op_canceled && errno != ECANCELED)) {
These three are all straightforward and would look simpler
in their simplified versions...
> 4.
>
> [qemu/target-arm/translate-a64.c:5729]: (style) Redundant condition:
> size<3. 'A && (!A || B)' is equivalent to 'A || B'
>
> if (size > 3
> || (size < 3 && is_q)
> || (size == 3 && !is_q)) {
...but I'm less sure about this one. I'm not even sure
what it's trying to suggest this should simplify to:
just dropping "size < 3" is obviously wrong, and the
condition format isn't "A && (!A || B)" either.
thanks
-- PMM
On 06/12/2015 05:01 AM, Peter Maydell wrote:
>> 4.
>>
>> [qemu/target-arm/translate-a64.c:5729]: (style) Redundant condition:
>> size<3. 'A && (!A || B)' is equivalent to 'A || B'
>>
>> if (size > 3
>> || (size < 3 && is_q)
>> || (size == 3 && !is_q)) {
>
> ...but I'm less sure about this one. I'm not even sure
> what it's trying to suggest this should simplify to:
> just dropping "size < 3" is obviously wrong, and the
> condition format isn't "A && (!A || B)" either.
Let's break it down into the 6 possibilities based on the binary *
ternary conditions being checked:
> 3, is_q => accept
> 3, !is_q => accept
== 3, is_q => reject
== 3, !is_q => accept
< 3, is_q => accept
< 3, !is_q => reject
Here's a shorter conditional with the same properties, but it's gross:
if (size > 3 || (is_q != (size == 3))) {
Too much mental thought to prove it accepts the same set of conditions.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
On 12 June 2015 at 14:03, Eric Blake <email address hidden> wrote:
> On 06/12/2015 05:01 AM, Peter Maydell wrote:
>
>>> 4.
>>>
>>> [qemu/target-arm/translate-a64.c:5729]: (style) Redundant condition:
>>> size<3. 'A && (!A || B)' is equivalent to 'A || B'
>>>
>>> if (size > 3
>>> || (size < 3 && is_q)
>>> || (size == 3 && !is_q)) {
>>
>> ...but I'm less sure about this one. I'm not even sure
>> what it's trying to suggest this should simplify to:
>> just dropping "size < 3" is obviously wrong, and the
>> condition format isn't "A && (!A || B)" either.
>
> Let's break it down into the 6 possibilities based on the binary *
> ternary conditions being checked:
>
>> 3, is_q => accept
>> 3, !is_q => accept
> == 3, is_q => reject
> == 3, !is_q => accept
> < 3, is_q => accept
> < 3, !is_q => reject
>
> Here's a shorter conditional with the same properties, but it's gross:
>
> if (size > 3 || (is_q != (size == 3))) {
>
> Too much mental thought to prove it accepts the same set of conditions.
Yeah, I think this is the kind of thing where I say "the compiler
should do this simplification if it cares enough" :-)
-- PMM
>These three are all straightforward and would look simpler
>in their simplified versions...
Agreed. The first 3 look valid candidates for simplification.
> 4.
>
> [qemu/target-arm/translate-a64.c:5729]: (style) Redundant condition:
> size<3. 'A && (!A || B)' is equivalent to 'A || B'
>
> if (size > 3
> || (size < 3 && is_q)
> || (size == 3 && !is_q)) {
>...but I'm less sure about this one.
Me too. Suggest regard as a false positive from the static analysis tool
and so leave the original code alone.
Patches have been committed:
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=f96fe6b5c27b9a66dba71
http://git.qemu.org/?p=qemu.git;a=commitdiff;h=5f333d79a4337b390fa41
Released with version 2.8
|