Re: [PATCH] kernel/groups.c: consider about NULL for 'group_info' in all related extern functions

Tejun Heo
2013-09-27T03:36:57Z

Hello, Chen.

On Fri, Sep 27, 2013 at 09:30:13AM +0800, Chen Gang wrote:
> As an integrator or large source code maintainer, we cannot only depend
> on testing, or tracing log, or some short directly causes; we also need
> find and solve issues by checking sub-systems' interface or documents.

Do you seriously think that you're the only one thinking the above?
You're repeating something obvious. The problem is that that's *all*
you're doing. Anyone who has *some* coding experience would know that
and you didn't move even an inch forward from there.

What you have done is obvious from your initial commit message, you
read some random piece of code, found something slightly inconsistent
with your concept of ideal construction, grepped a bit, and whipped up
that patch. It's such a perfect cliche of inexperience. It's
something immediately obvious and *exactly* why you were asked to
actually try out your hypothesis. It was supposed to serve two
purposes - either proving or disproving your patch && if your
knee-jerk patch was wrong, which I thought was likely given the
circumstances, making you think *why* you got it wrong and learn from
the experience.

When asked to actually verify your patch, you thought it was something
you were unfairly asked to do, and when your previous analysis was
shown to be wrong as so easily predicted, instead of thinking about
why you got that wrong and learning from the experience, you're now
just repeating the same crap in the opposite direction.

You're missing massive amount of steps between recognizing that
something is inconsistent and producing a proper improvement for it
and failing to recognize your shortcoming even when it failed right in
front of your face. You need to understand how the subsystem and code
actually work before making changes, study the callers and history of
the code especially if the code has been stable / stale for long
period of time, verify your assumptions using objective measures, and
present that in your patch. For a patch like this, preferably with
some risk analysis.

So, please take some time to mull over why your initial patch was
completely wrong and I didn't even have to read the code to predict
that your patch has high chance of being wrong. Now, you're doing the
*exactly* same thing in the opposite direction. You should be able to
recognize that there's something very wrong with that.

Thanks.

--
tejun