Hi, Peter Memishian wrote: > While doing some assorted cleanup in the UV gate, I noticed two minor > issues with the dladm_*_conf_field() APIs: > > 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. > 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. > So, does anyone have any issue with constifying the last argument to > dladm_set_conf_field() and removing the third argument from > dladm_get_conf_field()? > I'm not on the project team anymore, but, I'd say definitely do the first. As for the second, I'm on the fence. Some people might find it useful, but you do make a good argument for getting rid of it. Dan
