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

[Dominique Martinet]

SF Markus Elfring wrote on Tue, Jan 06, 2015 at 03:34:08PM +0100:
> > Actually just seen that this precise example is fixed along with more
> > similar code paths in subsequents (!) patchs of the set.
> 
> Do you refer to my update suggestions with a subject like "One function
> call less in v9fs_â" here?

Yes. Looking at the patchset as a whole (and under the assumption that
calling p9_client_clunk with NULL is harmful), your patchset first
introduces many such calls then proceeds onto fixing them.

Thinking a bit more, I think that instead of just changing the order of
patches the cleanest way would be to submit this as "reworking jump
labels, making NULL checks useless in the process".
Still only looking at v9fs_create, with your patch#2 reflowing,
p9_client_clunk can't be called with NULL unless a bug happened (either
p9_client_fcreate or p9_client_walk returning non-error but leaving ofid
or fid NULL), so it'd actually make sense to remove the quiet check in
favor of the warn/stack dump check at this point.

Is what I'm saying making sense?

> > It could actually be interesting to see if we could get all such
> > paths "fixed".
> 
> Would you like to see any more specific source code clean-up?
> Which kind of fine-tuning have you got in mind?

I was really just thinking that if the code flow is thought out the if's
can be removed harmlessly, but that it actually needs to be done the
other way around.
I honestly am not involved enough in kernel coding style changes or 9p
code as a whole to suggest anything else, sorry :)

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