jan damborsky wrote: > Hi Sarah, > > > On 04/06/09 16:18, Sarah Jelinek wrote: >> 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. > > Thank you very much for digging into this - > based on your observations it seems that it is save > to change the code. I will file bug for man page. > >> >> 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. > > Actually, ls_init_attr is not NULL only if debug mode is enabled - > otherwise it is initialized to NULL on line 1560. > If anything changes in that area (which seems unlikely at this point), > we would realize it soon :-) > Ah ok.. >> >> Bottom line.. I don't think we need to check for NULL before freeing >> the list. > > I have changed the code and removed checks for NULL. > > Thanks a lot !
You are welcome. sarah **** > > Jan >
