[v2] paride/pcd: return earlier when an error happens in pcd_atapi()
Commit Message
clang static analysis reports
drivers/block/paride/pcd.c:856:36: warning: The left operand of '&'
is a garbage value [core.UndefinedBinaryOperatorResult]
tocentry->cdte_ctrl = buffer[5] & 0xf;
~~~~~~~~~ ^
When the call to pcd_atapi() fails, buffer[] is in an unknown state,
so return early.
Signed-off-by: Tom Rix <trix@redhat.com>
---
v2: remove unused 'r' variable
---
drivers/block/paride/pcd.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
Comments
On 1/22/23 8:49 AM, Tom Rix wrote:
> clang static analysis reports
> drivers/block/paride/pcd.c:856:36: warning: The left operand of '&'
> is a garbage value [core.UndefinedBinaryOperatorResult]
> tocentry->cdte_ctrl = buffer[5] & 0xf;
> ~~~~~~~~~ ^
Has this one been compiled? I'm guessing not tested...
In any case, this code is going away hopefully shortly, so let's not
bother with changes like this.
On 1/22/23 12:49 PM, Jens Axboe wrote:
> On 1/22/23 8:49 AM, Tom Rix wrote:
>> clang static analysis reports
>> drivers/block/paride/pcd.c:856:36: warning: The left operand of '&'
>> is a garbage value [core.UndefinedBinaryOperatorResult]
>> tocentry->cdte_ctrl = buffer[5] & 0xf;
>> ~~~~~~~~~ ^
> Has this one been compiled? I'm guessing not tested...
>
> In any case, this code is going away hopefully shortly, so let's not
> bother with changes like this.
Going away soon would be nice, this is an old problem.
I did not bother with a fixes: tag because it was is when the repo was
created in 2005.
Tom
>
On Sun, Jan 22, 2023 at 01:49:00PM -0700, Jens Axboe wrote:
> On 1/22/23 8:49 AM, Tom Rix wrote:
> > clang static analysis reports
> > drivers/block/paride/pcd.c:856:36: warning: The left operand of '&'
> > is a garbage value [core.UndefinedBinaryOperatorResult]
> > tocentry->cdte_ctrl = buffer[5] & 0xf;
> > ~~~~~~~~~ ^
>
> Has this one been compiled? I'm guessing not tested...
>
> In any case, this code is going away hopefully shortly, so let's not
> bother with changes like this.
Look at the callers - the value left in entry is discarded if
->audio_ioctl(..., CDROMREADTOCENTRY, &entry) returns non-zero. Sure,
it's a nasal daemon territory, but realistically it's not going to be
caught by testing.
On 1/22/23 3:10 PM, Al Viro wrote:
> On Sun, Jan 22, 2023 at 01:49:00PM -0700, Jens Axboe wrote:
>> On 1/22/23 8:49 AM, Tom Rix wrote:
>>> clang static analysis reports
>>> drivers/block/paride/pcd.c:856:36: warning: The left operand of '&'
>>> is a garbage value [core.UndefinedBinaryOperatorResult]
>>> tocentry->cdte_ctrl = buffer[5] & 0xf;
>>> ~~~~~~~~~ ^
>>
>> Has this one been compiled? I'm guessing not tested...
>>
>> In any case, this code is going away hopefully shortly, so let's not
>> bother with changes like this.
>
> Look at the callers - the value left in entry is discarded if
> ->audio_ioctl(..., CDROMREADTOCENTRY, &entry) returns non-zero. Sure,
> it's a nasal daemon territory, but realistically it's not going to be
> caught by testing.
I don't expect anyone really to be able to test it, but v1 had a pretty
basic issue that would've surely triggered a compiler warning had it
been compiled.
@@ -825,14 +825,14 @@ static int pcd_audio_ioctl(struct cdrom_device_info *cdi, unsigned int cmd, void
struct cdrom_tochdr *tochdr =
(struct cdrom_tochdr *) arg;
char buffer[32];
- int r;
- r = pcd_atapi(cd, cmd, 12, buffer, "read toc header");
+ if (pcd_atapi(cd, cmd, 12, buffer, "read toc header"))
+ return -EIO;
tochdr->cdth_trk0 = buffer[2];
tochdr->cdth_trk1 = buffer[3];
- return r ? -EIO : 0;
+ return 0;
}
case CDROMREADTOCENTRY:
@@ -845,13 +845,13 @@ static int pcd_audio_ioctl(struct cdrom_device_info *cdi, unsigned int cmd, void
struct cdrom_tocentry *tocentry =
(struct cdrom_tocentry *) arg;
unsigned char buffer[32];
- int r;
cmd[1] =
(tocentry->cdte_format == CDROM_MSF ? 0x02 : 0);
cmd[6] = tocentry->cdte_track;
- r = pcd_atapi(cd, cmd, 12, buffer, "read toc entry");
+ if (pcd_atapi(cd, cmd, 12, buffer, "read toc entry"))
+ return -EIO;
tocentry->cdte_ctrl = buffer[5] & 0xf;
tocentry->cdte_adr = buffer[5] >> 4;
@@ -866,7 +866,7 @@ static int pcd_audio_ioctl(struct cdrom_device_info *cdi, unsigned int cmd, void
(((((buffer[8] << 8) + buffer[9]) << 8)
+ buffer[10]) << 8) + buffer[11];
- return r ? -EIO : 0;
+ return 0;
}
default: