> Hi Jan,
>
> Thanks and my reply in line.
>
> And also some style comments (not offend):
> 1, in libnwam_env.c, about the 1st argu: "nwam_env_svc_t **input".
> I think all "*input" can be replaced with "input".
Hmm, not really. we are expecting an array of "nwam_env_svc_t" pointers
here.
> 2, in libnwam_env.c, miss "(void)" before many
> nwam_tx_handle_free() and free().
> :)
>
will check and fix them.
> -----Original Message-----
> From: Zhenghui.Xie at Sun.COM
> Sent: 2007?09?13? 00:33
>> hi, Jia
>>
>> thanks for reviewing, my reply in line.
>>
>>> Hi Jan,
>>>
>>> Really change a lot of the "env" part! :)
>>>
>>
>> indeed. unfortunately :-)
>>
>>> 1, Do you think it's safe to put nwam_env_attr_t and nwam_env_svc_t
>>> in the libnwam.h file?
>>>
>>> If one can accest the pointer of that structure, one can walk
>>> through the entire list of svcs and attributes, or even one can
>>> modify it without invoke the interface by this lib.
>>> Also, if one add some invalid entities into the list, some uncertain
>>> behavior may be occured.
>>>
>>
>> Good question!
>>
>> env_svc_t is an svc item that doesn't contain next pointer, so user
>> should not be able
> Yes, you are right.
>> to access the next item. attr_t is designed to be a list. Basically,
>> libnwam returns a svc object to the user, and this svc object
>> contains a list of attributes. Does this make sense to you?
> But the attr_t list can still be access directly, right? But from the
> api we can see that it's really dangrous for a user to modify the
> attr_t list directly.
> For example, all the list operation is under the assumption that no
> duplicate items there (in other words, each item in the list has
> individual name). If one modify the list directly, there could be 2 or
> more items with the same name.
>
I'd have to think through a little more. But maybe we should pass a
duplicate copy of the svc obj and ask cb() to free the duplicate copy
before it returns.
About the duplicate name issue you mentioned, validation function should
handle it, though it has not finished yet.
>>
>>
>>> Compared to the ncp/ncu implementation, no list access there can be
>>> found in the libnwam.h, nwam_date_t also contains no pointer in the
>>> structure.
>>>
>>
>> yes. This is the difference between env and ncp/ncu. (And this is
>> what makes env design and implementation go back and forth several
>> times) We know what exactly are in ncp/ncu, but we don't know what
>> exactly env's attributes are.
>>
> struct nwam_data only take value but no pointer. Once the value
> modified, the only thing invovled is itself. So if there are some
> malicious code or some mis-operation in the call back function, very
> little harm it does. But if the list can be access directly, I think
> it's more dangerous.
I agree, which makes me tend to return a duplicate copy of the object to
the user.
>>
>>> This is also related to 2nd argument of the callback function (used
>>> in nwam_env_walk_props() and nwam_env_walk_svc()).
>>>
>>> 2, In function nwam_free_list_item() in file libnwam_util.c.
>>>
>>> 125 case (NWAM_SVC_LIST):
>>> 126 {
>>> 127 nwam_env_svclist_t *tmp = (nwam_env_svclist_t *)item;
>>> 128 (void) nwam_free_list_item(tmp->svc_obj, NWAM_SVC_OBJ);
>>> 129 break;
>>> 130 }
>>>
>>> "nwam_env_svclist_t *tmp" should also be freed after its obj freed.
>>>
>>
>> correct. I will fix this. Thanks!
>>
>>> Beside, as a test develop, I wonder if there are some update of doc
>>> or readme or man page after so many interface changes.
>>>
>>
>> yes, which is next on my task list. stay tuned. :-)
> Thanks a lot! With your great help, my life can be much easy :-)
>
it's on our project website now.
-Jan
