Re: [Qemu-devel] [PATCH] slirp: Fix error reported by static code analysis and remove wrong type casts

[Peter Maydell]

On 3 September 2012 21:34, Stefan Weil <address@hidden> wrote:
> Report from smatch:
> slirp/tcp_subr.c:127 tcp_respond(17) error:
>  we previously assumed 'tp' could be null (see line 124)
>
> Fix this by checking 'tp' before reading its elements.
>
> The type casts of pointers to long are not related to the smatch report
> but happened to be near that code. Those type casts are not allowed
> when sizeof(pointer) != sizeof(long).

I think these would be better in a separate patch.
> @@ -124,7 +124,7 @@ tcp_respond(struct tcpcb *tp, struct tcpiphdr *ti, struct 
> mbuf *m,
>         if (tp)
>                 win = sbspace(&tp->t_socket->so_rcv);
>          if (m == NULL) {
> -               if ((m = m_get(tp->t_socket->slirp)) == NULL)
> +               if (tp && (m = m_get(tp->t_socket->slirp)) == NULL)
>                         return;
>                 tlen = 0;
>                 m->m_data += IF_MAXLINKHDR;

This doesn't look quite right -- now if tp is NULL we will skip
the assignment to m and dereference a NULL pointer a few lines
further on, right?

I suspect that we need either to be passed our Slirp* explicitly rather
than via tp, or  we have to enforce that tcp_respond() is called with
a non-NULL struct tcpcb*. I think the only cases where tp can be non-NULL
at the moment are the two calls from the dropwithreset code in tcp_input().

-- PMM