Re: [PATCH 3/3] staging: lustre: Less function calls in class_register_type() after error detection

[Dan Carpenter]

These patches are often correct in the same way a stopped clock is
correct twice a day, but I reject the motivation/approach/patch
description.  Just because there is a sanity check does not mean we
should use it (ie, do an insane thing).  It hurts readability to hide
the NULL check.

On the other hand, half the time the NULL checks are superflous because
the pointer is never NULL.  The other half of the time the NULL checks
are there because the code uses one err style error handling.  Also the
error handling code should mirror the allocation code so I feel like it
should be:

	if (some_feature)
		foo = allocate();
	ret = frob();
	if (ret)
		goto free_foo;

free_foo:
	if (some_feature)  // <--- as opposed to if (foo) {
		free(foo);

So anyway I often don't like the original code, but the new code is
even worse and I have never heard a good motivation for these patches
besides that they were generated using a cool tool.  It's not a good
reason.  Other maintainers are accepting these patches so you are free
to send them there.

regards,
dan carpenter
--
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/