[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