thanks for the comments!

Anurag S. Maskey wrote:
>
>
> Alan Maguire wrote:
>>
>>
>> http://zhadum.east.sun.com/export/ws/amaguire/nwam1-fixes/webrev/
>>
>>
> General:
>
> setting the handle to NULL after freeing ensures that double freeing 
> won't segfault. Would it make sense to do this in the library, where 
> nwam_free() sets the handle to NULL?
I think it would.
>
>
> Also, one thing that I've noticed is that when NULL handles are passed 
> to the library (say with nwam_*_read()) and the read fails because the 
> entity does not exist, the handle is not NULL anymore. If a NULL 
> handle is passed, should it be set back to NULL when not returning 
> NWAM_SUCCESS. (nwamadm uses NULL handles after the code review and 
> every time _read(), _create(), _copy(), etc fails, handles have to be 
> set to NULL).
That's annoying - I'll fix this.
>
> nwamd/ncu.c:596 - what's the reason behind setting the user_enabled to 
> TRUE when the enabled property cannot be read?
>
It's for prioritized NCUs that don't have an enabled property -
user_enabled is probably a misleading name though.
> nwamd/ncu.c:1427-1430 - why not move the disabled NCU to disabled 
> state right away rather than online*?
>
The IP NCU may have interfaces plumbed, and going
directly to disabled would mean we don't run the
state machine and do the unplumb.
> There are a few cases where we check if a TRIGGERED_CHECK event is 
> already enqueued or not. I think all these these triggered check 
> events (NCU_CHECK and CHECK_CONDITIONS) should check if there's an 
> event already enqueued or not (moving nwamd_event_enqueued() call 
> inside the _nwamd_create_*_check_event() function).
>
Makes sense, I'll do that. Thanks again!

Alan

Reply via email to