On Thu, Jan 04, 2024 at 10:41:50AM +1000, Dave Airlie wrote: > On Thu, 4 Jan 2024 at 00:47, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote: > > > > Hi Dave, > > > > kernel test robot noticed the following build warnings: > > > > https://git-scm.com/docs/git-format-patch#_base_tree_information] > > > > url: https://github.com/intel-lab-lkp/linux/commits/Dave-Airlie/nouveau-gsp-drop-some-acpi-related-debug/20231222-180432 > > base: git://anongit.freedesktop.org/drm/drm-misc drm-misc-next > > patch link: https://lore.kernel.org/r/20231222043308.3090089-9-airlied%40gmail.com > > patch subject: [PATCH 08/11] nouveau/gsp: don't free ctrl messages on errors > > config: powerpc-randconfig-r071-20231226 (https://download.01.org/0day-ci/archive/20231227/202312271917.55xuDMdc-lkp@xxxxxxxxx/config) > > compiler: clang version 18.0.0git (https://github.com/llvm/llvm-project d3ef86708241a3bee902615c190dead1638c4e09) > > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > > the same patch/commit), kindly add following tags > > | Reported-by: kernel test robot <lkp@xxxxxxxxx> > > | Reported-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx> > > | Closes: https://lore.kernel.org/r/202312271917.55xuDMdc-lkp@xxxxxxxxx/ > > This is a false positive, I think the code is operating like I'd > expect, we maybe could restructure it to avoid this warning? > > The idea is you send an rpc msg, if there's a reply you get a reply, > if no reply you get NULL and if an error you get an error. > > So in the case you get an error or NULL you just want to return 0 for > the NULL as it's successful, and error otherwise. > > Would using PTR_ERR_OR_ZERO make smatch happy? (even if it's not > really what we want). Hm... You're using the API correctly. Linus has complained about this warning before but in new code over 90% of the warnings are correct. It's a high quality warning. I looked around for an explanation to see what the NULL meant but couldn't find it documented in the code. The NULL vs error pointer comes from a function pointer and it's not always clear where the documentation should be with a function pointer. So perhaps I missed it. Let's not use PTR_ERR_OR_ZERO. Perhaps I should introduce a PTR_ERR_OR_NULL() macro to silence this warning. But most of the code which does this correctly is in fs/ and they probably are like Linus and would be surprised to learn that people get it wrong... regards, dan carpenter