linux-kernel - Re: [PATCH 15/23] seq_file: switch over direct seq_read method calls to seq_read

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]

Message-ID: <87eep9rgqu.fsf@nanos.tec.linutronix.de>
Date:   Fri, 17 Jul 2020 23:09:13 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Christoph Hellwig <hch@....de>, Al Viro <viro@...iv.linux.org.uk>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Stephen Rothwell <sfr@...b.auug.org.au>
Cc:     Luis Chamberlain <mcgrof@...nel.org>,
        Matthew Wilcox <willy@...radead.org>,
        Kees Cook <keescook@...omium.org>,
        Iurii Zaikin <yzaikin@...gle.com>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH 15/23] seq_file: switch over direct seq_read method calls to seq_read_iter

Christoph Hellwig <hch@....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