[...]
>>> -static int host_pci_config_read(int pos, int len, uint32_t *val)
>>> +static void host_pci_config_read(int pos, int len, uint32_t *val,
>>> Error **errp)
>>> {
>>> char path[PATH_MAX];
>>> int config_fd;
>>> @@ -812,19 +812,19 @@ static int host_pci_config_read(int pos, int
>>> len, uint32_t *val)
>>> /* Access real host bridge. */
>>> int rc = snprintf(path, size,
>>> "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
>>> 0, 0, 0, 0, "config");
>>> - int ret = 0;
>>> if (rc >= size || rc < 0) {
>>> - return -ENODEV;
>>> + error_setg(&error_abort, "Invalid PCI device");
>>
>> I think a return statement is missing here.
>
> Well, error_setg(&error_abort) will never return, however checking the
> documentation again I noticed I forgot these recommendations:
>
> * Please don't error_setg(&error_fatal, ...), use error_report() and
> * exit(), because that's more obvious.
> * Likewise, don't error_setg(&error_abort, ...), use assert().
>
> abort() might be OK but a bit harsh, so I'll use error_report() + exit()
> here.
Actually g_strdup_printf() is cleaner.
>>> }
>>> config_fd = open(path, O_RDWR);
>>> if (config_fd < 0) {
>>> - return -ENODEV;
>>> + error_setg_errno(errp, errno, "Failed to open: %s", path);
>>> + return;
>>> }
>>> if (lseek(config_fd, pos, SEEK_SET) != pos) {
>>> - ret = -errno;
>>> + error_setg_errno(errp, errno, "Failed to seek: %s", path);
>>> goto out;
>>> }
>>> @@ -832,32 +832,31 @@ static int host_pci_config_read(int pos, int
>>> len, uint32_t *val)
>>> rc = read(config_fd, (uint8_t *)val, len);
>>> } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>>> if (rc != len) {
>>> - ret = -errno;
>>> + error_setg_errno(errp, errno, "Failed to read: %s", path);
>>> }
>>> out:
>>> close(config_fd);
>>> - return ret;
>>> }
[...]
