Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Christoph Hellwig <hch@lst.de>, Al Viro <viro@zeniv.linux.org.uk>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Stephen Rothwell <sfr@canb.auug.org.au>
Cc:     Luis Chamberlain <mcgrof@kernel.org>,
        Matthew Wilcox <willy@infradead.org>,
        Kees Cook <keescook@chromium.org>,
        Iurii Zaikin <yzaikin@google.com>,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH 15/23] seq_file: switch over direct seq_read method calls to seq_read_iter
In-Reply-To: <20200707174801.4162712-16-hch@lst.de>
References: <20200707174801.4162712-1-hch@lst.de> <20200707174801.4162712-16-hch@lst.de>
Date:   Fri, 17 Jul 2020 23:09:13 +0200
Message-ID: <87eep9rgqu.fsf@nanos.tec.linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Christoph Hellwig <hch@lst.de> writes:

> Switch over all instances used directly as methods using these sed
> expressions:
>
> sed -i -e 's/\.read\(\s*=\s*\)seq_read/\.read_iter\1seq_read_iter/g'

This sucks, really. I just got a patch against this converting the
changed version to DEFINE_SHOW_ATTRIBUTE(somefile) and thereby removing
the whole open coded gunk.

If we do a tree wide change like this, then can we pretty please use a
coccinelle script to convert all trivial instances to use
DEFINE_SHOW_ATTRIBUTE so we don't have to touch the same place over and
over.

Out of 375 places changed in your patch something about 2/3rd fall into
the trivial category:

static int debug_stats_open(struct inode *inode, struct file *filp)
{
	return single_open(filp, debug_stats_show, NULL);
}

static const struct file_operations debug_stats_fops = {
	.open		= debug_stats_open,
	.read		= seq_read,
	.llseek		= seq_lseek,
	.release	= single_release,
};

which can be replaced by:

DEFINE_SHOW_ATTRIBUTE(debug_stats);

removing 12 lines of gunk and one central place to do the iter change.

I'm pretty sure that quite some of the others which have only an
additional write function can be replaced by a new macro
DEFINE_RW_ATTRIBUTE() or such.

Needs some thought and maybe some cocci help from Julia, but that's way
better than this brute force sed thing which results in malformed crap
like this:

static const struct file_operations debug_stats_fops = {
	.open		= debug_stats_open,
	.read_iter		= seq_read_iter,
	.llseek		= seq_lseek,
	.release	= single_release,
};

and proliferates the copy and paste voodoo programming.

Thanks,

        tglx