On Mon, Mar 28, 2011 at 3:46 PM, Sage Weil <sage@xxxxxxxxxxxx> wrote: > Below is a patch that wraps the internal Objecter compound ObjectOperation > so that we can send compound operations to the OSDs via librados. > > The internal ObjectOperationImpl ends up being unnecessary; I just cast it > to the internal type directly. > > One weird thing I noticed is with the constructor. In Rados:: did > > + static ObjectOperation *operation_create(); > > and the user then deletes it when they're done (after using it for one or > more calls to operate() or aio_operate()). Is that the the approach we > want? Because right above that in the header is > > int ioctx_create(const char *name, IoCtx &pioctx); > I second TV's argument that returning a detailed error code is better than returning NULL or not-NULL. I also don't like calling conventions that always use dynamic memory allocation. Even on a shiny new 16-core system, there is still a global mutex around new()/free(). I also like putting stuff on the stack and using RAII. (I haven't looked at this new code enough to see whether RAII makes sense here though.) If we do go with the new/free based approach, I guess we need another function to call free() on it, because the library user might be using a different libc. cheers, Colin -- 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
