[Ocfs2-tools-devel] [PATCH] fix wrap assignment for extended slot map in ocfs2_store_slot_map()

[Coly Li]

Joel Becker Wrote:
> On Mon, Jun 15, 2009 at 04:06:00PM +0800, Coly Li wrote:
>> in ocfs2_store_slot_map(), no matter extended slot map is used, only wrap.mw_map
>> is initialized, which will introduce a wild memory accessing when extended slot
>> map is used.
> 
> 	The slot_map_wrapper structure is a union.  That is, the address
> of mw_map and mw_map_extended is the same.  There's no wild memory
> access because merely accessing the union's addresses doesn't select
> which portion of the union is used later.
> 
>> @@ -334,7 +334,12 @@ errcode_t ocfs2_store_slot_map(ocfs2_filesys *fs,
>>  	ret = ocfs2_malloc0(bytes, &slot_map_buf);
>>  	if (ret)
>>  		return ret;
>> -	wrap.mw_map = (struct ocfs2_slot_map *)slot_map_buf;
>> +
>> +	if (extended)
>> +		wrap.mw_map_extended =
>> +			(struct ocfs2_slot_map_extened *)slot_map_buf;
>> +	else
>> +		wrap.mw_map = (struct ocfs2_slot_map *)slot_map_buf;
> 
> 	Your change makes explicit what C considers implicit.  It's a
> union of one pointer.  That pointer can be named mw_map_extended or
> mw_map, but it's the same pointer.  So the code
> 
> 	wrap.mw_map = (struct ocfs2_slot_map *)slot_map_buf;
> 
> is equivalent to:
> 
> 	(char *)wrap.mw_map = slot_map_buf;
> 
> It is just setting the pointer, with casts making sure that the compiler
> does not complain.  Higher up in ocfs2_store_slot_map(), we make sure
> the buffer is big enough and we zero it out.
> 	So, NAK, the code is correct as-is.
> 
Hi Joel,

I toke slot_map_wrapper as a structure other than union in my patch. That's how
my (incorrect) conclusion came.
Yes, the code is correct. Thanks for the patient explaining :)


-- 
Coly Li
SuSE Labs