On 14/12/12 19:18, Gert Doering wrote:
> Hi,
>
> next one: mtcp.c, about line 470:
>
> unsigned int flags = MTP_NONE;
>
> if (TUN_OUT(c))
> flags |= MTP_TUN_OUT;
> if (LINK_OUT(c))
> flags |= MTP_LINK_OUT;
>
> switch (flags)
> {
> case MTP_TUN_OUT|MTP_LINK_OUT:
> case MTP_TUN_OUT:
> newaction = TA_TUN_WRITE;
> break;
> case MTP_LINK_OUT:
> newaction = TA_SOCKET_WRITE;
> break;
> case MTP_NONE:
> if (mi && socket_read_residual (c->c2.link_socket))
> newaction = TA_SOCKET_READ_RESIDUAL;
> else
> multi_tcp_set_global_rw_flags (m, mi);
> break;
> default:
> {
> struct gc_arena gc = gc_new ();
> msg (M_FATAL, "MULTI TCP: multi_tcp_post bad state, mi=%s flags=%d",
> multi_instance_string (mi, false, &gc),
> flags);
> gc_free (&gc);
> break;
> }
> }
>
>
>
> coverity is complaining that the "default:" clause can never be reached,
> as all the possible values for "flags" are covered - and it's obviously
> right. It's not something critical, more a "code cleanup" thing.
>
> (The whole usage of switch/case here reeks a bit, given that I think the
> code would be simpler by directly setting "newaction" depending on
> "if (TUN_OUT(c))" etc., not bothering with "flags" in the first place)
>
> Cleanup in 2.4?Does it hurt to have this "dead code"? Agreed, if all the flags are covered, we'll never see 'default' in action. But is it possible that these flags can be extended at some point in the future, which we need to account for? I somehow find confidence in having this "trap" with msg(M_FATAL,...) for future scenarios. Not because I strongly believe it will be needed, but more like a precaution (as in "Yes, we have thought about this") -- kind regards, David Sommerseth
signature.asc
Description: OpenPGP digital signature
