On Sat, Jun 28, 2014 at 12:20 PM, Josh Triplett <josh@xxxxxxxxxxxxxxxx> wrote: > Except that "filter" has an "int channel" (signed), so it can > successfully test "channel < 0" and return early; it'll never call > ni_stc_dma_channel_select_bitfield(channel) with a negative number. > > I do agree that this code should sort out the signedness of its types, > but in this case I don't think the bad shift can actually happen, making > this a false positive. Ah, I see. I agree that the warning happen on the dead code. However, the root cause for the warning is not because sparse can not figure out the value of "channel". Sparse actually correctly does the inline and figure out "channel" is 4294967295. The root cause of your complain against sparse is that it happen on the dead code. I don' think the patch is the right fix for this problem though. This patch try to silence a warning purely because it can happen on dead code, otherwise perfectly valid warning. My point is that, it can happen on dead code is not a valid reason to weaken the warning. Sparse can give any possible warning on dead code. e.g. On the dead code path, you can construct an address space warning. Do we weakling the address space warning purely because it can happen on the dead code as well? Of course not. The same apply to any other sparse warnings. So if there is a bug, It is sparse did not know exactly which part of the code is dead on AST level. If you enable the "#if 0" inside expand_if_statement(), sparse can actually correctly expand "if (channel < 0)" into "return". Of course, it still can't handle jump into dead code situation, which is the reason that piece of code is not enabled in the first place. It needs more work. I think that is a better approach than just random weakening some warnings on dead code path. Chris -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
