Message ID | 20240228163723.1775791-7-zhao1.liu@linux.intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Cleanup up to fix missing ERRP_GUARD() for error_prepend() | expand |
On Thu, Feb 29, 2024 at 12:37:13AM +0800, Zhao Liu wrote: > From: Zhao Liu <zhao1.liu@intel.com> > > As the comment in qapi/error, passing @errp to error_prepend() requires > ERRP_GUARD(): > > * = Why, when and how to use ERRP_GUARD() = > * > * Without ERRP_GUARD(), use of the @errp parameter is restricted: > ... > * - It should not be passed to error_prepend(), error_vprepend() or > * error_append_hint(), because that doesn't work with &error_fatal. > * ERRP_GUARD() lifts these restrictions. > * > * To use ERRP_GUARD(), add it right at the beginning of the function. > * @errp can then be used without worrying about the argument being > * NULL or &error_fatal. > > ERRP_GUARD() could avoid the case when @errp is the pointer of > error_fatal, the user can't see this additional information, because > exit() happens in error_setg earlier than information is added [1]. > > In nvme.c, there're 3 functions passing @errp to error_prepend() > without ERRP_GUARD(): > - nvme_init_queue() > - nvme_create_queue_pair() > - nvme_identify() > > All these 3 functions take their @errp parameters from the > nvme_file_open(), which is a BlockDriver.bdrv_nvme() method and its > @errp points to its caller's local_err. > > Though these 3 cases haven't trigger the issue like [1] said, to > follow the requirement of @errp, add missing ERRP_GUARD() at their > beginning. > > [1]: Issue description in the commit message of commit ae7c80a7bd73 > ("error: New macro ERRP_GUARD()"). > > Cc: Stefan Hajnoczi <stefanha@redhat.com> > Cc: Fam Zheng <fam@euphon.net> > Cc: "Philippe Mathieu-Daudé" <philmd@linaro.org> > Cc: Kevin Wolf <kwolf@redhat.com> > Cc: Hanna Reitz <hreitz@redhat.com> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com> > --- > block/nvme.c | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
diff --git a/block/nvme.c b/block/nvme.c index 0a0a0a6b36cd..3a3c6da73d29 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -168,6 +168,7 @@ static QemuOptsList runtime_opts = { static bool nvme_init_queue(BDRVNVMeState *s, NVMeQueue *q, unsigned nentries, size_t entry_bytes, Error **errp) { + ERRP_GUARD(); size_t bytes; int r; @@ -221,6 +222,7 @@ static NVMeQueuePair *nvme_create_queue_pair(BDRVNVMeState *s, unsigned idx, size_t size, Error **errp) { + ERRP_GUARD(); int i, r; NVMeQueuePair *q; uint64_t prp_list_iova; @@ -535,6 +537,7 @@ static int nvme_admin_cmd_sync(BlockDriverState *bs, NvmeCmd *cmd) /* Returns true on success, false on failure. */ static bool nvme_identify(BlockDriverState *bs, int namespace, Error **errp) { + ERRP_GUARD(); BDRVNVMeState *s = bs->opaque; bool ret = false; QEMU_AUTO_VFREE union {