[openib-general] [PATCHv5] osm: partition manager force policy

Eitan Zahavi eitan at mellanox.co.il
Tue Jun 20 23:10:10 PDT 2006


Hi Hal,

Thanks for applying the patch.

Regarding the issues :

Hal Rosenstock wrote:
>>+
>>+	CL_ASSERT( p_pkey_tbl );
> 
> 
> Should the other routines also assert on this or should this be
> consistent with the others ?
Yes it should b consistent.
Normally I add assertion on OUT parameters such that a "misuse" is caught.
The idea is that parameters provided by reference are more likely to be passed
by mistake as NULL. So I would remove the assert on p_key_tbl.
> 
> 
>>+	CL_ASSERT( p_block_idx != NULL );
>>+	CL_ASSERT( p_pkey_idx != NULL );
> 
> 
> There is no p_pkey_idx parameter. I presume this should be p_pkey_index.
Ooops - this means the code will not compile in debug mode !
I see you fixed that.
> 
> 
> Also, two things about osm_pkey_mgr.c:
> 
> Was there a need to reorder the routines ? This broke the diff so it had
> to be done largely by hand.
I reordered to to be defined in the order used.
Already agree with Sasha that I should have done that on separate patch.
> 
> Also, it would have been nice not to mix the format changes with the
> substantive changes. Try to keep it to "one thought per patch".
OK.
> 
> This patch has been applied with cosmetic changes. We will go from
> here...
Thanks

Eitan




More information about the general mailing list