linux-kernel - Re: [PATCH 2/3] mmc: Export ios settings for a host through debugfs

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: <20080628154700.426a422c@hskinnemo-gx745.norway.atmel.com>
Date:	Sat, 28 Jun 2008 15:47:00 +0200
From:	Haavard Skinnemoen <haavard.skinnemoen@...el.com>
To:	Pierre Ossman <drzeus-mmc@...eus.cx>
Cc:	linux-kernel@...r.kernel.org,
	Haavard Skinnemoen <hskinnemoen@...el.com>
Subject: Re: [PATCH 2/3] mmc: Export ios settings for a host through debugfs

Pierre Ossman <drzeus-mmc@...eus.cx> wrote:
> On Thu, 26 Jun 2008 13:09:48 +0200
> Haavard Skinnemoen <haavard.skinnemoen@...el.com> wrote:

> > +static void mmc_remove_ios_debugfs(struct mmc_ios *ios)
> > +{
> > +	if (ios->dbg_root) {
> 
> You can save a bit of indentation if you do:
> 
> 	if (!ios->dbg_root)
> 		return;

Good point.

> > +static int mmc_add_ios_debugfs(struct mmc_ios *ios, struct dentry *parent)
> > +{
> > +	struct dentry *dir;
> > +
> > +	dir = debugfs_create_dir("ios", parent);
> > +	if (!dir)
> > +		return -EBUSY;	/* or whatever */
> 
> Is it undefined what a NULL return means here? I would have expected an
> ERRPTR or ENOMEM.

Yeah...debugfs_create_dir() may return an ERRPTR only if debugfs is
disabled. All other errors return NULL.

We wouldn't be here unless the caller managed to create the host
directory, so checking IS_ERR() is redundant. Maybe ENOMEM would make
more sense, but I could also be that the directory already exists,
hence EBUSY is appropriate. We really don't know as long as debugfs
keeps us in the dark...

> > +#ifdef CONFIG_MMC_DEBUG_FS
> > +	struct dentry		*dbg_root;
> > +	struct dentry		*dbg_clock;
> > +	struct dentry		*dbg_vdd;
> > +	struct dentry		*dbg_bus_mode;
> > +	struct dentry		*dbg_chip_select;
> > +	struct dentry		*dbg_power_mode;
> > +	struct dentry		*dbg_bus_width;
> > +	struct dentry		*dbg_timing;
> > +#endif
> 
> Can't we use debugfs' own bookkeeping to keep track of them? Saves us a
> lot of noise in these structures.

You mean d_subdirs in struct dentry? I guess we could do that...though
I was sort of trying not to dig too deply into VFS internals...

Haavard
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/