Hello everybody,
While looking into Coverity ID 1226913 I ran into the following piece of
code at drivers/uwb/i1480/dfu/phy.c:99:
99static
100int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16 srcaddr,
size_t size)
101{
102 int result;
103 struct i1480_cmd_mpi_read *cmd = i1480->cmd_buf;
104 struct i1480_evt_mpi_read *reply = i1480->evt_buf;
105 unsigned cnt;
106
107 memset(i1480->cmd_buf, 0x69, 512);
108 memset(i1480->evt_buf, 0x69, 512);
109
110 BUG_ON(size > (i1480->buf_size - sizeof(*reply)) / 3);
111 result = -ENOMEM;
112 cmd->rccb.bCommandType = i1480_CET_VS1;
113 cmd->rccb.wCommand = cpu_to_le16(i1480_CMD_MPI_READ);
114 cmd->size = cpu_to_le16(3*size);
115 for (cnt = 0; cnt < size; cnt++) {
116 cmd->data[cnt].page = (srcaddr + cnt) >> 8;
117 cmd->data[cnt].offset = (srcaddr + cnt) & 0xff;
118 }
119 reply->rceb.bEventType = i1480_CET_VS1;
120 reply->rceb.wEvent = i1480_CMD_MPI_READ;
121 result = i1480_cmd(i1480, "MPI-READ", sizeof(*cmd) + 2*size,
122 sizeof(*reply) + 3*size);
123 if (result < 0)
124 goto out;
125 if (reply->bResultCode != UWB_RC_RES_SUCCESS) {
126 dev_err(i1480->dev, "MPI-READ: command execution failed:
%d\n",
127 reply->bResultCode);
128 result = -EIO;
129 }
130 for (cnt = 0; cnt < size; cnt++) {
131 if (reply->data[cnt].page != (srcaddr + cnt) >> 8)
132 dev_err(i1480->dev, "MPI-READ: page inconsistency
at "
133 "index %u: expected 0x%02x, got
0x%02x\n", cnt,
134 (srcaddr + cnt) >> 8,
reply->data[cnt].page);
135 if (reply->data[cnt].offset != ((srcaddr + cnt)
& 0x00ff))
136 dev_err(i1480->dev, "MPI-READ: offset
inconsistency at "
137 "index %u: expected 0x%02x, got
0x%02x\n", cnt,
138 (srcaddr + cnt) & 0x00ff,
139 reply->data[cnt].offset);
140 data[cnt] = reply->data[cnt].value;
141 }
142 result = 0;
143out:
144 return result;
145}
The issue is that the value store in variable _result_ at line 128 is
overwritten by the one stored at line 142, before it can be used.
My question is if the original intention was to return this value
inmediately after the assignment at line 128, something like in the
following patch:
index 3b1a87d..1ac8526 100644
--- a/drivers/uwb/i1480/dfu/phy.c
+++ b/drivers/uwb/i1480/dfu/phy.c
@@ -126,6 +126,7 @@ int i1480_mpi_read(struct i1480 *i1480, u8 *data, u16
srcaddr, size_t size)
dev_err(i1480->dev, "MPI-READ: command execution failed:
%d\n",
reply->bResultCode);
result = -EIO;
+ goto out;
}
for (cnt = 0; cnt < size; cnt++) {
if (reply->data[cnt].page != (srcaddr + cnt) >> 8)
What do you think?
I'd really appreciate any comment on this.
