Re: [PATCH 00 of 23][-mm] Unionfs: Stackable Namespace Unification Filesystem

Re: [PATCH 00 of 23][-mm] Unionfs: Stackable Namespace Unification Filesystem

[Andrew Morton]

On Thu, 12 Oct 2006 02:18:37 -0400
Josef "Jeff" Sipek <jsipek@cs.sunysb.edu> wrote:

> This set of patches constitutes Unionfs version 2.0.

So we need to get this reviewed.

Christoph and I are pretty much the only people who do filesystem
reviewing, despite the fact that we have hoards of filesystem developers
out there busily doing their own stuff (and that includes the unionfs
developers, I might add).

Our review-to-development ratio is wildly out of whack and I don't know how
to fix that.

Meanwhile, from a quick scan I'd say that unionfs is much, much too lightly
commented for a review to be particularly effective.   Please work on that.

Re: [PATCH 00 of 23][-mm] Unionfs: Stackable Namespace Unification Filesystem

[Pekka Enberg]

On 10/12/06, Andrew Morton <akpm@osdl.org> wrote:
> Meanwhile, from a quick scan I'd say that unionfs is much, much too lightly
> commented for a review to be particularly effective.   Please work on that.

Could use some basic coding style fixes too.

- Move assignments outside of if statement expression "if (err = foo_bar())"
- No C99-style comments "//"
- Use struct kmem_cache instead of the deprecated kmem_cache_t
- Don't use function-like macros as the left hand side of assignment
expression "itohi_ptr(inode) = kzalloc(size, GFP_KERNEL);". It's much
better to open-code the assignment or introduce a setter function
(e.g. inode_set_hiptr).
- Kill wrappers (e.g. unionfs_kill_block_super can be replaced with
generic_shutdown_super)

                                                  Pekka

Re: [PATCH 00 of 23][-mm] Unionfs: Stackable Namespace Unification Filesystem

[Jan Engelhardt]

>> Meanwhile, from a quick scan I'd say that unionfs is much, much too
>> lightly
>> commented for a review to be particularly effective.   Please work on
>> that.
>
> - Kill wrappers (e.g. unionfs_kill_block_super can be replaced with
> generic_shutdown_super)

Possibly the unionfs_kill_block_super function was once written with 
more than just generic_shutdown_super in mind. Just a guess, though.


	-`J'
--