On Tue, 5 Nov 2013, Dan Carpenter wrote:
> Hello Alan Stern,
>
> The patch b35c5009bbf6: "USB: EHCI: create per-TT bandwidth tables"
> from Oct 11, 2013, leads to the following
> static checker warning: "drivers/usb/host/ehci-sched.c:1377
> reserve_release_iso_bandwidth()
> error: 'tt' dereferencing possible ERR_PTR()"
>
> drivers/usb/host/ehci-sched.c
> 1375 tt = find_tt(stream->ps.udev);
> ^^^^^^^^^^^^^^^^^^^^^^^^
> This can return ERR_PTR(-ENOMEM).
>
> 1376 if (sign > 0)
> 1377 list_add_tail(&stream->ps.ps_list,
> &tt->ps_list);
> 1378 else
> 1379 list_del(&stream->ps.ps_list);
> 1380
> 1381 for (i = uframe >> 3; i < EHCI_BANDWIDTH_FRAMES;
> 1382 i += stream->ps.bw_period)
> 1383 tt->bandwidth[i] += tt_usecs;
>
> Also:
>
> drivers/usb/host/ehci-sched.c
> 257 /* FS/LS bus bandwidth */
> 258 if (tt_usecs) {
> 259 tt = find_tt(qh->ps.udev);
> ^^^^^^^^^^^^^^^^^^^^^^^^
> 260 if (sign > 0)
> 261 list_add_tail(&qh->ps.ps_list, &tt->ps_list);
> 262 else
> 263 list_del(&qh->ps.ps_list);
> 264
> 265 for (i = start_uf >> 3; i < EHCI_BANDWIDTH_FRAMES;
> 266 i += qh->ps.bw_period)
> 267 tt->bandwidth[i] += tt_usecs;
Ah, yes. I sort of wondered if a static checker would be able to pick
up on this.
The overall pattern is not too uncommon:
f(x) returns x->ptr, if x->ptr is non-NULL. Otherwise it
attempts to allocate a new structure and makes x->ptr point
to it. If memory isn't available, f(x) returns
ERR_PTR(-ENOMEM).
g() calls f(x) twice, but checks the result with IS_ERR() only
the first time. It is known that nothing will change or
deallocate x->ptr between the two calls.
Short of adding redundant tests, how would you suggest this be handled?
Do nothing and live with a false positive report from the checker?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
