On Thu, 8 Mar 2012, Josh Durgin wrote: > On 03/08/2012 06:28 PM, Noah Watkins wrote: > > > > On Mar 8, 2012, at 6:16 PM, Greg Farnum wrote: > > > > > > /* Returns a configuration value as a string. > > > > * If len is positive, that is the maximum number of bytes we'll write > > > > into the > > > > * buffer. If len == -1, we'll call malloc() and set *buf. > > > > * Returns 0 on success, error code otherwise. Returns ENAMETOOLONG if > > > > the > > > > * buffer is too short. */ > > > > > > Ah. Two things from that: > > > 1) "If len == -1". Which it isn't, here. > > > 2) There is no voodoo, that just isn't going to work. Either it was > > > created incorrectly in reference to md_config_t::get_val, or else code got > > > moved around without updating that documentation. :/ Options including > > > char ** (as you said), char *& (ewww), or?not doing that? > > > > > > Anybody have thoughts on the right solution? > > > > I am partial to APIs that put the burden on the caller to free/expand the > > buffer in response to -ENAMETOOLONG. /2cents > > That sounds good to me. That comment exactly describes the behavior of > md_config_t::get_val(). It would also be cleaner if len didn't have a special > meaning and md_config_t::get_val() used strings instead of char*. It already > uses strings internally, and only the librados C++ api actually uses len=-1. On a related note, I think we should be keeping a list somewhere of all the little annoying API changes we want to make, so that when we DO go and bump the SONAME we can cram it all in at once. Something like http://ceph.newdream.net/wiki/API_change_wishlist ? sage -- To unsubscribe from this list: send the line "unsubscribe ceph-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
