On Mon, Nov 18, 2019 at 12:25:14PM +0800, Green Wan wrote: > Hi Dan, > > Thanks for reaching out. I do have the same question before. If I > remember it correctly, the 'chan' is returned by to_sf_pdma_chan() > which contains 'dchan'. 'chan' s unlikely to be NULL since it's > container. to_sf_pdma_chan() is really a no-op when you read it carefully. "chan" and "dchan" are equivalent. Smatch parses it correctly. You are obviously correct that people should check the original "dchan" instead of the returned "chan", but I feel like some people are clever enough to know when container_of() is a no-op and deliberately check the returned value... These are sorts of people you don't want to get into a debate with because they're an ultra annoying blend of clever and dumb. My other comment here is that Smatch doesn't warn about inconsistent NULL checking when the pointer is provably non-NULL. In this case, the only caller that Smatch doesn't parse correctly is ioat_dma_self_test(). For all the other callers it knows that "dchan" is non-NULL. In ioat_dma_self_test() the code looks like: 330 /* Start copy, using first DMA channel */ 331 dma_chan = container_of(dma->channels.next, struct dma_chan, 332 device_node); Smatch says that both "dma->channels.next" and "dma_chan" are complete unknowns. I could probably safely mark dma_chan as a valid pointer in this instance which would make the warning disappear... Maybe I will add a check to see if "dma->channels.next" can be controlled by the user. regards, dan carpenter
