On Sat, 2013-03-02 at 23:35 +0100, Paul Bolle wrote:
[...]
> 0) I've had another look at the relevant code in v3.8.2-rc1. It can be
> summarized like this:
>
> static int xen_vbd_translate()
> {
> [...]
> int rc = -EACCES;
>
> if ([...])
> goto out;
> [...]
>
> [p]req->dev = vbd->pdevice;
> [p]req->bdev = vbd->bdev;
> [...]
>
> out:
> return rc;
> }
>
> static int dispatch_rw_block_io()
> {
> struct phys_req preq;
> [...]
>
> preq.sector_number = req->u.rw.sector_number;
> preq.nr_sects = 0;
> [...]
>
> for ([...]) {
> [...]
> preq.nr_sects += seg[i].nsec;
> }
>
> if (xen_vbd_translate(&preq, blkif, operation) != 0) {
> pr_debug(DRV_PFX "access denied: %s of [%llu,%llu] on
> dev=%04x\n",
> operation == READ ? "read" : "write",
> preq.sector_number,
> preq.sector_number + preq.nr_sects, preq.dev);
> goto [...];
> }
> [...]
> }
>
> 1) So if xen_vbd_translate() fails, it can return before setting
> preq.dev. That makes the call of pr_debug() use an uninitialized value,
> doesn't it?Oh yes, so it's a completely valid warning in this case! > Does inlining xen_vbd_translate() and/or > dispatch_rw_block_io() generate code were that can't happen anymore? > (Both functions being static they probably are inlined.) > > 2) And even if inlining does generate code where this can't happen, > isn't it enough that preq.dev can be used uninitialized if no code were > inlined? If gcc inlines a function call, it analyses data flow between the two functions. Otherwise it assumes that the called function will initialise any variable it's given a pointer to, and the warning doesn't appear. (That's my experience, anyway.) Ben. -- Ben Hutchings Computers are not intelligent. They only think they are.
signature.asc
Description: This is a digitally signed message part
