On Thu, May 07, 2020 at 09:40:22AM -0400, Brian Foster wrote: > On Thu, May 07, 2020 at 02:27:18PM +0200, Christoph Hellwig wrote: > > On Fri, May 01, 2020 at 09:34:31AM -0400, Brian Foster wrote: > > > > + default: > > > > + xfs_inode_verifier_error(ip, -EFSCORRUPTED, __func__, dip, > > > > + sizeof(*dip), __this_address); > > > > + return -EFSCORRUPTED; > > > > + } > > > > > > Can we fix this function up to use an error variable and return error at > > > the end like xfs_iformat_attr_work() does? Otherwise nice cleanup.. > > > > What would the benefit of a local variable be here? It just adds a > > little extra code for no real gain. > > > > It looks like the variable is already defined, it's just not used > consistently. The only extra code are break statements in the switch and > a return statement at the end of the function, which currently looks odd > without it IMO. As of this patch there is no local error variable. Later on it gets added, but only used in a single place for the fork verifier. I find functions that basically are a big switch statement and return from each case pretty normal.