Re: VFS minor improvements policy

[Jan Kara <jack@suse.cz>]

Hi!

On Sat 14-03-26 14:24:23, Jori Koolstra wrote:
> I submitted a patch a few days ago about fixing the docstring of
> hash_name().  This is not a reminder of that patch, but today I came
> across something else in the VFS code, while trying to implement a
> mkdirat_fd syscall, that I thought might be slightly improved. We have
> this anonymous enum:
> 
>    enum {LAST_NORM, LAST_ROOT, LAST_DOT, LAST_DOTDOT};
> 
> but it is also used as an out parameter in filename_parentat() through
> int *type.  Now you have to look up what this int might hold, so I wonder
> should this struct not have an explicit type that filename_parentat()
> uses?

I guess this is a bit matter of personal preference. I personally would
prefer explicitely named enum in this case and use it where appropriate but
Al? apparently preferred brewity and used 'int' everywhere. The borderline
between "useless typedefs" and "documentation by types" is blurry. I'd say
trying to "fix" this would do more harm than good.

> However, my question is really this: what is the stance of vfs
> maintainers/reviewer for submitting these kind of patches? While going
> through the code in more depth I've noticed wrong/unhelpful docstrings
> here and there, and other minor "issues". Now, Greg KH has taught me to
> be really conservative with what counts as an improvement, for instance
> to not sneak-in white-space fixes in a patch.

If the documentation / comment is pointless or even wrong, that's
definitely worth a fix. They are just not treated with particularly high
priority but they are definitely needed.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR