linux-kernel - Re: [patch 2/2] reiser4: new export ops

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:	Wed, 28 Nov 2007 10:43:01 +0100
From:	Christoph Hellwig <hch@....de>
To:	Edward Shishkin <edward@...esys.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Reiserfs-list <reiserfs-devel@...r.kernel.org>,
	bfields@...i.umich.edu, Neil Brown <neilb@...e.de>,
	Christoph Hellwig <hch@....de>,
	Linux kernel mailing list <linux-kernel@...r.kernel.org>
Subject: Re: [patch 2/2] reiser4: new export ops

This code looks a little confusing to me..

>   */
>  static char *decode_inode(struct super_block *s, char *addr,
>  			  reiser4_object_on_wire * obj)
> @@ -41,7 +42,8 @@
>  	fplug = file_plugin_by_disk_id(reiser4_get_tree(s), (d16 *) addr);
>  	if (fplug != NULL) {
>  		addr += sizeof(d16);
> -		obj->plugin = fplug;
> +		if (obj)
> +			obj->plugin = fplug;

You are adding quite a few of those if (obj) clauses.  I can't see a
reason for that - care to explain?  The new aops should not disallow for
any functionality that has been there before.

> static struct dentry *reiser4_decode_fh(struct super_block *super, __u32 *fh,
> +					int len, int fhtype, int parent)
>  {
>  	reiser4_context *ctx;
>  	reiser4_object_on_wire object;
>  	char *addr;
>  
>  	ctx = reiser4_init_context(super);
>  	if (IS_ERR(ctx))
> @@ -80,25 +77,19 @@
>  	assert("vs-1482",
>  	       fhtype == FH_WITH_PARENT || fhtype == FH_WITHOUT_PARENT);
>  
> 	addr = (char *)fh;
> 
> 	object_on_wire_init(&object);
> +
> +	if (parent)
> +		/* skip first onwire object */
> +		addr = decode_inode(super, addr, NULL);
> 	if (!IS_ERR(addr)) {
> +		addr = decode_inode(super, addr, &object);
> 		if (!IS_ERR(addr)) {
> 			struct dentry *d;
> 
> +			d = reiser4_get_dentry(super, &object);

I'd suggest to directly poke into the place where the parent handle
is stored.  XFS used a similar construct to the decode_inode helper,
but with the new aops it's faster and easier to read if you just have
a helper on how many bytes to skip.  Did you take a look at how the
various other filesystem handle the export ops?

> --- linux-2.6.23-mm1/fs/reiser4/dscale.c.orig
> +++ linux-2.6.23-mm1/fs/reiser4/dscale.c
> @@ -126,6 +126,24 @@

How are the changes to all these other files related to the export
operations changes?

-
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/