On Mon, Nov 7, 2011 at 13:43, Gregory Farnum <gregory.farnum@xxxxxxxxxxxxx> wrote: > This generally looks good to me, but that return statement is weirding > me out a bit. I guess you're trying to compress the information about > the number of returned addresses, and whether or not you got all the > addresses, into that one value? It seems more idiomatic to me to send > back the number of filled-in addresses, or an error code if there > wasn't enough space for all of them. > I'm not sure if that idiom is worth applying here, but it was my one > thought about the interface. :) The value in returning the number that *would have been written* is in that now the caller can reallocate a buffer of exactly the right size (barring races). Like snprintf() and friends. With just a generic -ENOBUFS/-EMSGSIZE/whatever you cram in there, the caller must do a series of increasing guesses. But yes, the function is tricky to use right. Gets a 3 on the Rusty scale I'd say.. http://ozlabs.org/~rusty/index.cgi/tech/2008-03-30.html Maybe instead of extern "C" int ceph_get_file_stripe_address(struct ceph_mount_info *cmount, int fh, loff_t offset, struct sockaddr_storage *addr, int naddr) it should be extern "C" int ceph_get_file_stripe_address(struct ceph_mount_info *cmount, int fh, loff_t offset, struct sockaddr_storage *addr, int naddr, int *naddr_out) and the return value is just 0 or -errno, gets -ESOMETHING if all entries won't fit, and naddr_out (if not NULL) is set to be the number of entries that would have been returned. More moving parts but harder to get wrong by forgetting to check ret < naddr. -- 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
