Am 14.01.2016 um 13:14 hat Daniel P. Berrange geschrieben: > On Wed, Jan 13, 2016 at 07:42:20PM +0100, Kevin Wolf wrote: > > Am 12.01.2016 um 19:56 hat Daniel P. Berrange geschrieben: > > > +static ssize_t qcow2_fde_header_read_func(QCryptoBlock *block, > > > + size_t offset, > > > + uint8_t *buf, > > > + size_t buflen, > > > + Error **errp, > > > + void *opaque) > > > +{ > > > + BlockDriverState *bs = opaque; > > > + BDRVQcow2State *s = bs->opaque; > > > + ssize_t ret; > > > + > > > + if ((offset + buflen) > s->fde_header.length) { > > > + error_setg_errno(errp, EINVAL, > > > + "Request for data outside of extension header"); > > > > error_setg_errno() with a constant errno doesn't look very useful. > > Better use plain error_setg() in such cases. > > I wasn't too sure - I figured since the block layer seems to > propagate errno's around alot, that I ought to report an > errno here, but will happiyl drop it.
error_setg_errno() doesn't keep the error code around for the callers to inspect, but just adds the error string to the message. And you already have a much more useful error message than "Invalid argument". > > > + return -1; > > > > Here returning -EINVAL could be useful, I'm not sure what your crypto > > API requires. At least you seem to be returning -errno below and mixing > > -1 and -errno is probably a bad idea. > > The crypto API doesn't deal with errno's at all - it uses the > Error object exclusively, so yeah, I can drop it from the > place below. Then you could probably just make the function void. I genereally prefer to use only one mechanism to return errors instead of both an int return value and an Error** argument, but there are many places in qemu which use both. So whatever feels right to you. Kevin