Re: [V9fs-developer] [PATCH 1/8] fs/9p: Deletion of unnecessary checks before the function call "p9_client_clunk"

[Dominique Martinet]

Dominique Martinet wrote on Tue, Jan 06, 2015 at 09:12:53AM +0100:
> Simple example, without even looking at the multiple possible
> interactions with multiple clients hammering the server, can be taken
> from v9fs_create in fs/9p/vfs_inode.c
> https://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/tree/fs/9p/vfs_inode.c?id=d8282ea05ad119247122de23db7d48ad6098cfa2#n652
> 
> jumping to label error from a failure in p9_client_fcreate (e.g. EPERM
> or something perfectly valid) will, with your patch, call clunk with fid
> == NULL and thus generate a stack trace, while it is a perfectly normal
> code path that should only return to userspace with EPERM.

Actually just seen that this precise example is fixed along with more
similar code paths in subsequents (!) patchs of the set.

It could actually be interesting to see if we could get all such paths
"fixed".
If so, I believe this way will ultimately lead to cleaner code, but I'd
rather see taken all other patches first and keep this one in v9fs-devel
branch for a bit longer if this makes sense... Well, I guess there is a
test window before the merge to linus tree anyway, but given the number
of active 9P users it could be useful.


(All other patches in the set look good to me at second glance, now.
To answer Dan's mail, none of these are bug fix as far as I've seen,
just avoiding unnecessary checks for null or calls+check/warn depending
on whether you apply patch #1 first)

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