linux-kernel - Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver internals

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 08 May 2018 13:42:33 -0700
From:   Joe Perches <joe@...ches.com>
To:     Gary R Hook <gary.hook@....com>, "Hook, Gary" <ghook@....com>,
        kbuild test robot <lkp@...el.com>
Cc:     kbuild-all@...org, iommu@...ts.linux-foundation.org,
        joro@...tes.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 1/2] iommu - Enable debugfs exposure of IOMMU driver
 internals

On Tue, 2018-05-08 at 15:07 -0500, Gary R Hook wrote:
> On 05/08/2018 01:48 PM, Joe Perches wrote:
> > On Tue, 2018-05-08 at 12:08 -0500, Hook, Gary wrote:
> > > On 5/7/2018 6:47 PM, kbuild test robot wrote:
> > > > 
> > > > All error/warnings (new ones prefixed by >>):
> > > > 
> > > >      In file included from include/linux/intel-iommu.h:32:0,
> > > >                       from drivers/gpu/drm/i915/i915_drv.h:41,
> > > >                       from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
> > > >      include/linux/iommu.h: In function 'iommu_debugfs_new_driver_dir':
> > > > > > include/linux/iommu.h:706:8: error: parameter name omitted
> > > > 
> > > >       struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > > >              ^~~~~~
> > > >      In file included from include/linux/intel-iommu.h:32:0,
> > > >                       from drivers/gpu/drm/i915/i915_drv.h:41,
> > > >                       from drivers/gpu/drm/i915/i915_oa_bxt.c:31:
> > > > > > include/linux/iommu.h:706:8: warning: control reaches end of non-void function [-Wreturn-type]
> > > > 
> > > >       struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > > >              ^~~~~~
> > > > 
> > > > vim +706 include/linux/iommu.h
> > > > 
> > > >      700	
> > > >      701	#ifdef CONFIG_IOMMU_DEBUGFS
> > > >      702	void iommu_debugfs_setup(void);
> > > >      703	struct dentry *iommu_debugfs_new_driver_dir(char *);
> > > >      704	#else
> > > >      705	static inline void iommu_debugfs_setup(void) {}
> > > >    > 706	struct dentry *iommu_debugfs_new_driver_dir(char *) {};
> > > >      707	#endif
> > > >      708	
> > > 
> > > I have no problems with adding parameter names. But
> > > scripts/checkpatch.pl doesn't seem to check for this, nor require it.
> > > Should checkpatch be updated?
> > 
> > I'm pretty sure that's not feasible.
> 
> Ugh. This is a definition, not a declaration. My bad. Which is likely 
> why I decided to apologize up front.
> 
> > And when the compiler tells you you've stuffed up some
> > syntactical bit, why should checkpatch duplicate the
> > output error message too?
> 
> Well, that's the point: neither the 4.8 nor 5.4 compiler complained 
> about this.

Perhaps because CONFIG_IOMMU_DEBUGFS was set in the .config
for all the compilation previously performed?

>  Not as an error, despite the fact that (now that I read what 
> is actually here, as opposed to what I think is there) this is wrong. 
> Had an error message been emitted, and the make stopped, I would have 
> figure this out before embarrassing myself in front of the entire interwebs.

There's no reason for that figuring out to be necessary.
Linux compilation complexity is pretty high and almost
no one understands it completely.

cheers, Joe