Sowmini.Varadhan at Sun.COM wrote:
>>> typedef dladm_status_t pd_getf_t(const char *, char **, uint_t *);
>>> typedef dladm_status_t pd_setf_t(const char *, val_desc_t *, uint_t);
>>>
>>> i.e., why don't they both take val_desc_t * as the second arg, which
>>> seems to be more general?
>>>
>> I did this based on the original libwladm code. I also thought about making
>> them consistent when I did the change, but I found that taking val_desc_t *
>> as the argument of pd_getf_t only add more complexities. It is because we
>> only expect the result from the xxx_get() function to be in the form of
>> string, and the caller allocates the memory to keep the result makes things
>> easier.
>
> Speaking only from eyeballing the code, so I could well be missing something:
>
> Couldn't this be achieved by making a wrapper function whose vd_val
> contains the needed info needed to get the prop_val and the val_cnt? Something
> along the lines of:
> typedef struct foo_s {
> char **prop_val;
> int *val_cnt
> }foo_t;
>
> foo_t prop;
> val_desc_t v = {NULL, &prop};
>
> It creates extra indirections in the wrapper function, but
> val_desc_t is a bit more flexible for me, because I need to pass in
> 4 pieces of info: link_name, prop_name, prop_val, an val_cnt
>
I don't quite understand the above example. As I understand, if there is
only val_desc_t, then val_cnt is 1.
That is, if you have more than one values to set:
- the pd_setf_t() function will take a val_desc_t array as the second
argument, and the count of the values to be the third argument.
- the pd_getf_t() function. The caller will take a pointer to a string array
as the second argument and a pointer to a count as the third argument. So
far it always take a string array size of MAX_PROP_VALS as the input to
decrease the complexity.
>
>>> Also, the third arg in the pd_setf_t signature looks a bit confusing.
>>> Does this indicate the count of number of elements in vd_val?
>>> or is it the number of val_desc_t elements itself? (Could use some
>>> comments in the code, since its hard to remember these semantics)
>>>
>> It's the number of the val_desc_t elements. As the type of vd_val (void *)
>
> Ok, could you please stick a little comment in there to this effect?
>
Sure.
- Cathy
