[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 02/15] qga: Consistently name Error ** object
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH v2 02/15] qga: Consistently name Error ** objects errp, and not err |
Date: |
Tue, 29 Apr 2014 15:41:57 -0500 |
User-agent: |
alot/0.3.4 |
Quoting Markus Armbruster (2014-04-28 15:27:41)
> Signed-off-by: Markus Armbruster <address@hidden>
> Reviewed-by: Eric Blake <address@hidden>
> ---
> qga/commands-posix.c | 209
> ++++++++++++++++++++++++++-------------------------
> qga/commands-win32.c | 115 ++++++++++++++--------------
> qga/commands.c | 4 +-
> qga/vss-win32.c | 4 +-
> qga/vss-win32.h | 2 +-
> 5 files changed, 170 insertions(+), 164 deletions(-)
>
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index 935a4ec..e49c7da 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -53,7 +53,7 @@ extern char **environ;
> #endif
> #endif
>
> -static void ga_wait_child(pid_t pid, int *status, Error **err)
> +static void ga_wait_child(pid_t pid, int *status, Error **errp)
> {
> pid_t rpid;
>
> @@ -64,14 +64,15 @@ static void ga_wait_child(pid_t pid, int *status, Error
> **err)
> } while (rpid == -1 && errno == EINTR);
>
> if (rpid == -1) {
> - error_setg_errno(err, errno, "failed to wait for child (pid: %d)",
> pid);
> + error_setg_errno(errp, errno, "failed to wait for child (pid: %d)",
> + pid);
> return;
> }
>
> g_assert(rpid == pid);
> }
>
> -void qmp_guest_shutdown(bool has_mode, const char *mode, Error **err)
> +void qmp_guest_shutdown(bool has_mode, const char *mode, Error **errp)
> {
> const char *shutdown_flag;
> Error *local_err = NULL;
> @@ -86,7 +87,7 @@ void qmp_guest_shutdown(bool has_mode, const char *mode,
> Error **err)
> } else if (strcmp(mode, "reboot") == 0) {
> shutdown_flag = "-r";
> } else {
> - error_setg(err,
> + error_setg(errp,
> "mode is invalid (valid values are:
> halt|powerdown|reboot");
> return;
> }
> @@ -103,23 +104,23 @@ void qmp_guest_shutdown(bool has_mode, const char
> *mode, Error **err)
> "hypervisor initiated shutdown", (char*)NULL, environ);
> _exit(EXIT_FAILURE);
> } else if (pid < 0) {
> - error_setg_errno(err, errno, "failed to create child process");
> + error_setg_errno(errp, errno, "failed to create child process");
> return;
> }
>
> ga_wait_child(pid, &status, &local_err);
> if (local_err) {
> - error_propagate(err, local_err);
> + error_propagate(errp, local_err);
> return;
> }
>
> if (!WIFEXITED(status)) {
> - error_setg(err, "child process has terminated abnormally");
> + error_setg(errp, "child process has terminated abnormally");
> return;
> }
>
> if (WEXITSTATUS(status)) {
> - error_setg(err, "child process has failed to shutdown");
> + error_setg(errp, "child process has failed to shutdown");
> return;
> }
>
> @@ -234,7 +235,7 @@ static int64_t guest_file_handle_add(FILE *fh, Error
> **errp)
> return handle;
> }
>
> -static GuestFileHandle *guest_file_handle_find(int64_t id, Error **err)
> +static GuestFileHandle *guest_file_handle_find(int64_t id, Error **errp)
> {
> GuestFileHandle *gfh;
>
> @@ -245,7 +246,7 @@ static GuestFileHandle *guest_file_handle_find(int64_t
> id, Error **err)
> }
> }
>
> - error_setg(err, "handle '%" PRId64 "' has not been found", id);
> + error_setg(errp, "handle '%" PRId64 "' has not been found", id);
> return NULL;
> }
>
> @@ -275,7 +276,7 @@ static const struct {
> };
>
> static int
> -find_open_flag(const char *mode_str, Error **err)
> +find_open_flag(const char *mode_str, Error **errp)
> {
> unsigned mode;
>
> @@ -292,7 +293,7 @@ find_open_flag(const char *mode_str, Error **err)
> }
>
> if (mode == ARRAY_SIZE(guest_file_open_modes)) {
> - error_setg(err, "invalid file open mode '%s'", mode_str);
> + error_setg(errp, "invalid file open mode '%s'", mode_str);
> return -1;
> }
> return guest_file_open_modes[mode].oflag_base | O_NOCTTY | O_NONBLOCK;
> @@ -303,7 +304,7 @@ find_open_flag(const char *mode_str, Error **err)
> S_IROTH | S_IWOTH)
>
> static FILE *
> -safe_open_or_create(const char *path, const char *mode, Error **err)
> +safe_open_or_create(const char *path, const char *mode, Error **errp)
> {
> Error *local_err = NULL;
> int oflag;
> @@ -370,11 +371,12 @@ safe_open_or_create(const char *path, const char *mode,
> Error **err)
> }
> }
>
> - error_propagate(err, local_err);
> + error_propagate(errp, local_err);
> return NULL;
> }
>
> -int64_t qmp_guest_file_open(const char *path, bool has_mode, const char
> *mode, Error **err)
> +int64_t qmp_guest_file_open(const char *path, bool has_mode, const char
> *mode,
> + Error **errp)
> {
> FILE *fh;
> Error *local_err = NULL;
> @@ -387,7 +389,7 @@ int64_t qmp_guest_file_open(const char *path, bool
> has_mode, const char *mode, E
> slog("guest-file-open called, filepath: %s, mode: %s", path, mode);
> fh = safe_open_or_create(path, mode, &local_err);
> if (local_err != NULL) {
> - error_propagate(err, local_err);
> + error_propagate(errp, local_err);
> return -1;
> }
>
> @@ -398,14 +400,14 @@ int64_t qmp_guest_file_open(const char *path, bool
> has_mode, const char *mode, E
> ret = fcntl(fd, F_GETFL);
> ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
> if (ret == -1) {
> - error_setg_errno(err, errno, "failed to make file '%s' non-blocking",
> + error_setg_errno(errp, errno, "failed to make file '%s'
> non-blocking",
> path);
> fclose(fh);
> return -1;
> }
>
> - handle = guest_file_handle_add(fh, err);
> - if (error_is_set(err)) {
> + handle = guest_file_handle_add(fh, errp);
> + if (error_is_set(errp)) {
> fclose(fh);
> return -1;
> }
> @@ -414,9 +416,9 @@ int64_t qmp_guest_file_open(const char *path, bool
> has_mode, const char *mode, E
> return handle;
> }
>
> -void qmp_guest_file_close(int64_t handle, Error **err)
> +void qmp_guest_file_close(int64_t handle, Error **errp)
> {
> - GuestFileHandle *gfh = guest_file_handle_find(handle, err);
> + GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
> int ret;
>
> slog("guest-file-close called, handle: %" PRId64, handle);
> @@ -426,7 +428,7 @@ void qmp_guest_file_close(int64_t handle, Error **err)
>
> ret = fclose(gfh->fh);
> if (ret == EOF) {
> - error_setg_errno(err, errno, "failed to close handle");
> + error_setg_errno(errp, errno, "failed to close handle");
> return;
> }
>
> @@ -435,9 +437,9 @@ void qmp_guest_file_close(int64_t handle, Error **err)
> }
>
> struct GuestFileRead *qmp_guest_file_read(int64_t handle, bool has_count,
> - int64_t count, Error **err)
> + int64_t count, Error **errp)
> {
> - GuestFileHandle *gfh = guest_file_handle_find(handle, err);
> + GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
> GuestFileRead *read_data = NULL;
> guchar *buf;
> FILE *fh;
> @@ -450,7 +452,7 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle,
> bool has_count,
> if (!has_count) {
> count = QGA_READ_COUNT_DEFAULT;
> } else if (count < 0) {
> - error_setg(err, "value '%" PRId64 "' is invalid for argument count",
> + error_setg(errp, "value '%" PRId64 "' is invalid for argument count",
> count);
> return NULL;
> }
> @@ -459,7 +461,7 @@ struct GuestFileRead *qmp_guest_file_read(int64_t handle,
> bool has_count,
> buf = g_malloc0(count+1);
> read_count = fread(buf, 1, count, fh);
> if (ferror(fh)) {
> - error_setg_errno(err, errno, "failed to read file");
> + error_setg_errno(errp, errno, "failed to read file");
> slog("guest-file-read failed, handle: %" PRId64, handle);
> } else {
> buf[read_count] = 0;
> @@ -477,13 +479,14 @@ struct GuestFileRead *qmp_guest_file_read(int64_t
> handle, bool has_count,
> }
>
> GuestFileWrite *qmp_guest_file_write(int64_t handle, const char *buf_b64,
> - bool has_count, int64_t count, Error
> **err)
> + bool has_count, int64_t count,
> + Error **errp)
> {
> GuestFileWrite *write_data = NULL;
> guchar *buf;
> gsize buf_len;
> int write_count;
> - GuestFileHandle *gfh = guest_file_handle_find(handle, err);
> + GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
> FILE *fh;
>
> if (!gfh) {
> @@ -496,7 +499,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle,
> const char *buf_b64,
> if (!has_count) {
> count = buf_len;
> } else if (count < 0 || count > buf_len) {
> - error_setg(err, "value '%" PRId64 "' is invalid for argument count",
> + error_setg(errp, "value '%" PRId64 "' is invalid for argument count",
> count);
> g_free(buf);
> return NULL;
> @@ -504,7 +507,7 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle,
> const char *buf_b64,
>
> write_count = fwrite(buf, 1, count, fh);
> if (ferror(fh)) {
> - error_setg_errno(err, errno, "failed to write to file");
> + error_setg_errno(errp, errno, "failed to write to file");
> slog("guest-file-write failed, handle: %" PRId64, handle);
> } else {
> write_data = g_malloc0(sizeof(GuestFileWrite));
> @@ -518,9 +521,9 @@ GuestFileWrite *qmp_guest_file_write(int64_t handle,
> const char *buf_b64,
> }
>
> struct GuestFileSeek *qmp_guest_file_seek(int64_t handle, int64_t offset,
> - int64_t whence, Error **err)
> + int64_t whence, Error **errp)
> {
> - GuestFileHandle *gfh = guest_file_handle_find(handle, err);
> + GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
> GuestFileSeek *seek_data = NULL;
> FILE *fh;
> int ret;
> @@ -532,7 +535,7 @@ struct GuestFileSeek *qmp_guest_file_seek(int64_t handle,
> int64_t offset,
> fh = gfh->fh;
> ret = fseek(fh, offset, whence);
> if (ret == -1) {
> - error_setg_errno(err, errno, "failed to seek file");
> + error_setg_errno(errp, errno, "failed to seek file");
> } else {
> seek_data = g_new0(GuestFileSeek, 1);
> seek_data->position = ftell(fh);
> @@ -543,9 +546,9 @@ struct GuestFileSeek *qmp_guest_file_seek(int64_t handle,
> int64_t offset,
> return seek_data;
> }
>
> -void qmp_guest_file_flush(int64_t handle, Error **err)
> +void qmp_guest_file_flush(int64_t handle, Error **errp)
> {
> - GuestFileHandle *gfh = guest_file_handle_find(handle, err);
> + GuestFileHandle *gfh = guest_file_handle_find(handle, errp);
> FILE *fh;
> int ret;
>
> @@ -556,7 +559,7 @@ void qmp_guest_file_flush(int64_t handle, Error **err)
> fh = gfh->fh;
> ret = fflush(fh);
> if (ret == EOF) {
> - error_setg_errno(err, errno, "failed to flush file");
> + error_setg_errno(errp, errno, "failed to flush file");
> }
> }
>
> @@ -596,7 +599,7 @@ static void free_fs_mount_list(FsMountList *mounts)
> /*
> * Walk the mount table and build a list of local file systems
> */
> -static void build_fs_mount_list(FsMountList *mounts, Error **err)
> +static void build_fs_mount_list(FsMountList *mounts, Error **errp)
> {
> struct mntent *ment;
> FsMount *mount;
> @@ -605,7 +608,7 @@ static void build_fs_mount_list(FsMountList *mounts,
> Error **err)
>
> fp = setmntent(mtab, "r");
> if (!fp) {
> - error_setg(err, "failed to open mtab file: '%s'", mtab);
> + error_setg(errp, "failed to open mtab file: '%s'", mtab);
> return;
> }
>
> @@ -645,7 +648,7 @@ const char *fsfreeze_hook_arg_string[] = {
> "freeze",
> };
>
> -static void execute_fsfreeze_hook(FsfreezeHookArg arg, Error **err)
> +static void execute_fsfreeze_hook(FsfreezeHookArg arg, Error **errp)
> {
> int status;
> pid_t pid;
> @@ -658,7 +661,7 @@ static void execute_fsfreeze_hook(FsfreezeHookArg arg,
> Error **err)
> return;
> }
> if (access(hook, X_OK) != 0) {
> - error_setg_errno(err, errno, "can't access fsfreeze hook '%s'",
> hook);
> + error_setg_errno(errp, errno, "can't access fsfreeze hook '%s'",
> hook);
> return;
> }
>
> @@ -673,24 +676,24 @@ static void execute_fsfreeze_hook(FsfreezeHookArg arg,
> Error **err)
> execle(hook, hook, arg_str, NULL, environ);
> _exit(EXIT_FAILURE);
> } else if (pid < 0) {
> - error_setg_errno(err, errno, "failed to create child process");
> + error_setg_errno(errp, errno, "failed to create child process");
> return;
> }
>
> ga_wait_child(pid, &status, &local_err);
> if (local_err) {
> - error_propagate(err, local_err);
> + error_propagate(errp, local_err);
> return;
> }
>
> if (!WIFEXITED(status)) {
> - error_setg(err, "fsfreeze hook has terminated abnormally");
> + error_setg(errp, "fsfreeze hook has terminated abnormally");
> return;
> }
>
> status = WEXITSTATUS(status);
> if (status) {
> - error_setg(err, "fsfreeze hook has failed with status %d", status);
> + error_setg(errp, "fsfreeze hook has failed with status %d", status);
> return;
> }
> }
> @@ -698,7 +701,7 @@ static void execute_fsfreeze_hook(FsfreezeHookArg arg,
> Error **err)
> /*
> * Return status of freeze/thaw
> */
> -GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
> +GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
> {
> if (ga_is_frozen(ga_state)) {
> return GUEST_FSFREEZE_STATUS_FROZEN;
> @@ -711,7 +714,7 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **err)
> * Walk list of mounted file systems in the guest, and freeze the ones which
> * are real local file systems.
> */
> -int64_t qmp_guest_fsfreeze_freeze(Error **err)
> +int64_t qmp_guest_fsfreeze_freeze(Error **errp)
> {
> int ret = 0, i = 0;
> FsMountList mounts;
> @@ -723,14 +726,14 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err)
>
> execute_fsfreeze_hook(FSFREEZE_HOOK_FREEZE, &local_err);
> if (local_err) {
> - error_propagate(err, local_err);
> + error_propagate(errp, local_err);
> return -1;
> }
>
> QTAILQ_INIT(&mounts);
> build_fs_mount_list(&mounts, &local_err);
> if (local_err) {
> - error_propagate(err, local_err);
> + error_propagate(errp, local_err);
> return -1;
> }
>
> @@ -740,7 +743,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err)
> QTAILQ_FOREACH_REVERSE(mount, &mounts, FsMountList, next) {
> fd = qemu_open(mount->dirname, O_RDONLY);
> if (fd == -1) {
> - error_setg_errno(err, errno, "failed to open %s",
> mount->dirname);
> + error_setg_errno(errp, errno, "failed to open %s",
> mount->dirname);
> goto error;
> }
>
> @@ -756,7 +759,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **err)
> ret = ioctl(fd, FIFREEZE);
> if (ret == -1) {
> if (errno != EOPNOTSUPP) {
> - error_setg_errno(err, errno, "failed to freeze %s",
> + error_setg_errno(errp, errno, "failed to freeze %s",
> mount->dirname);
> close(fd);
> goto error;
> @@ -779,7 +782,7 @@ error:
> /*
> * Walk list of frozen file systems in the guest, and thaw them.
> */
> -int64_t qmp_guest_fsfreeze_thaw(Error **err)
> +int64_t qmp_guest_fsfreeze_thaw(Error **errp)
> {
> int ret;
> FsMountList mounts;
> @@ -790,7 +793,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
> QTAILQ_INIT(&mounts);
> build_fs_mount_list(&mounts, &local_err);
> if (local_err) {
> - error_propagate(err, local_err);
> + error_propagate(errp, local_err);
> return 0;
> }
>
> @@ -829,21 +832,21 @@ int64_t qmp_guest_fsfreeze_thaw(Error **err)
> ga_unset_frozen(ga_state);
> free_fs_mount_list(&mounts);
>
> - execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, err);
> + execute_fsfreeze_hook(FSFREEZE_HOOK_THAW, errp);
>
> return i;
> }
>
> static void guest_fsfreeze_cleanup(void)
> {
> - Error *err = NULL;
> + Error *errp = NULL;
Surprisingly, I actually got this one right the first time around.
Well, I guess Error *local_err is the convention for these, so maybe
not...
Looks good otherwise.
- [Qemu-devel] [PATCH v2 00/15] qmp qga: Purge error_is_set(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 04/15] qmp: Consistently name Error ** objects errp, and not err, Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 03/15] qga: Consistently name Error ** objects errp, and not err, Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 01/15] qmp hmp: Consistently name Error * objects err, and not errp, Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 02/15] qga: Consistently name Error ** objects errp, and not err, Markus Armbruster, 2014/04/28
- Re: [Qemu-devel] [PATCH v2 02/15] qga: Consistently name Error ** objects errp, and not err,
Michael Roth <=
- [Qemu-devel] [PATCH v2 05/15] error: Consistently name Error ** objects errp, and not err, Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 08/15] qapi: Drop redundant, unclean error_is_set(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 13/15] qemu-option: Clean up fragile use of error_is_set(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 11/15] qga: Clean up fragile use of error_is_set(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 10/15] qapi: Clean up fragile use of error_is_set(), Markus Armbruster, 2014/04/28
- [Qemu-devel] [PATCH v2 15/15] qmp: Don't use error_is_set() to suppress additional errors, Markus Armbruster, 2014/04/28