jan damborsky wrote: > Hi Sarah, > > > On 04/06/09 15:18, Sarah Jelinek wrote: >> jan damborsky wrote: >>> Hi Joe, >>> >>> >>> On 04/06/09 13:57, Joseph J VLcek wrote: >>>> Hi Jan, >>>> >>>> Thank you for making the changes. >>>> >>>> Sundar's question about invoking ls_init() made me realize one more >>>> issue. >>>> >>>> >>>> usr/src/cmd/auto-install/auto_install.c >>>> ---------------------------------------- >>>> >>>> 1628 if (ls_init_attr != NULL) >>>> >>>> I believe this check, done on line 1628 for ls_init_attr != NULL, >>>> should also be done before line: >>>> >>>> 1623 nvlist_free(ls_init_attr); >>> >>> You are right - good point. I have added that check. >>> >> Actually, the check for NULL isn't necessary. If you look at the >> nvlist_free() code, it checks for NULL and does the right thing. >> Either way is fine with me, I just wanted to point this out. > > This is a good point. I would prefer to remove those > checks. But I am not sure we could rely on this behavior > since man page for nvlist_free(3NVPAIR) doesn't mention > this. Please let me know what you think. Hi Jan,
I looked at the arc case to see if they documented this behavior, they don't specifically document that if it is a NULL value no operation occurs. This case was filed in 2000 and this code has been in there for a while. I think that we can rely on this behavior since it has been for this long. It is my opinion it isn't likely to change. You could file a man page bug so that this gets documented properly, much like free(NULL) behavior is documented. Also, looking at your code, it doesn't seem to me that we would ever have a case where the list could be non-NULL at any point you are checking. Even if ls_init() failed it doesn't NULL and free the passed in list. Bottom line.. I don't think we need to check for NULL before freeing the list. thanks, sarah **** sarah *** > > thank you very much for review ! > Jan >
