Re: [PATCH] Use of getblk differs between locations

[Glauber de Oliveira Costa <glommer@br.ibm.com>]

Some of them can be wrong, but they're not that random.

Let's discuss each one of them

In the changes made in affs.h, I returned NULL because it seemed to be
the some-went-wrong behaviour on these functions

affs_getzeroblk, for example, tests some conditions. The buffer being
NULL is just another test.

In the code you commented, I thought that we get the same case testing
from or to conditions, and thus, it would be correct to threat them in
the same way.

in minix, ext2 and ext3 code, I changed the alloc_branch functions ,
because they already has some code that is prepared to recover from a
failed allocation

In the others, I've just put my hands on code that would execute inside
conditionals using struct members, to prevent dereferencing a NULL pointer. 
As I said, there is a lot of code that has the problem, and are untouched. 
If I was being at random, they would also figuring out here.

I don't think it will necessarily hide the problems (although it can be doing 
it in some of them), because __getblk_slow will produce a lot of noise
in the case something did get wrong.


glauber

On Tue, Oct 11, 2005 at 01:05:03AM +0100, Al Viro wrote:
> On Mon, Oct 10, 2005 at 09:07:34PM -0300, Glauber de Oliveira Costa wrote:
> >  	if (!bh)
> >  		return -EIO;
> >  	new = sb_getblk(sb, to);
> > +	if (!new)
> > +		return -EIO;
> 
> You've just introduced a leak here, obviously.
> 
> Please, read the code before "fixing" that stuff; slapping returns at random
> and hoping that it will help is not a good way to deal with that - the only
> thing you achieve is hiding the problem.
> 
> The same goes for the rest of patch - in each case it's not obvious that your
> changes are correct.
> 

-- 
=====================================
Glauber de Oliveira Costa
IBM Linux Technology Center - Brazil
glommer@br.ibm.com
=====================================