> > 1. dladm_set_conf_field() has the following prototype: > > > > dladm_status_t > > dladm_set_conf_field(dladm_conf_t conf, const char *attr, > > dladm_datatype_t type, void *attrval) > > > > Note the final argument of `attrval'. As best I can tell, this > > should be `const void *attrval' -- and because it's not, the > > callers often have to cast away const, bloating the call sites. > > > > Agreed. This was an oversight on my part when I wrote the API.
OK, I'll make this change. > > 2. dladm_get_conf_field() has the following prototype: > > > > dladm_status_t > > dladm_get_conf_field(dladm_conf_t conf, const char *attr, > > dladm_datatype_t *type, void *attrval, size_t attrsz) > > > > While this signature provides a certain symmetry with > > dladm_set_conf_field(), every caller already knows the type > > (otherwise, they wouldn't know how to fill in `attrsz'), and > > thus passes NULL for the third argument. Again, this bloats > > the call sites. > > I added the third argument as the result of the design review comments. > I believe Michael Hunter made the suggestion that adding that > argument would make certain automated processing easier. I remember > thinking it wouldn't hurt anything, so I added it. You're right that it doesn't cause architectural problems, but I can't see how one could use it successfully given the attrsz issue. Thanks! -- meem
