[pacman-dev] [PATCH] lib/util: call _alpm_log before setting handle->pm_errno
This is an unfortunate chain of events. RET_ERR and RET_ERR_VOID will eventually call CHECK_HANDLE, which resets the handle's pm_errno member. Dan probably had a reason for doing this, so we merely switch the order of operations in the RET_ERR macros to avoid stomping on our pm_errno. Signed-off-by: Dave Reisner <d@falconindy.com> --- Awesome bug. Here's what happens when the DB is locked: $ sudo pacman -Sy :: Synchronizing package databases... error: failed to init transaction (unexpected error) I'm still not sure why CHECK_HANDLE will always reset handle->pm_errno. Seems like it's some sort of lazy initialization that should be getting done elsewhere. There may be a better way to solve this, if the reset isn't actually necessary. lib/libalpm/util.h | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 0549c81..778e20f 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -61,12 +61,14 @@ #define ASSERT(cond, action) do { if(!(cond)) { action; } } while(0) -#define RET_ERR_VOID(handle, err) do { (handle)->pm_errno = (err); \ +#define RET_ERR_VOID(handle, err) do { \ _alpm_log(handle, PM_LOG_DEBUG, "returning error %d from %s : %s\n", err, __func__, alpm_strerror(err)); \ + (handle)->pm_errno = (err); \ return; } while(0) -#define RET_ERR(handle, err, ret) do { (handle)->pm_errno = (err); \ +#define RET_ERR(handle, err, ret) do { \ _alpm_log(handle, PM_LOG_DEBUG, "returning error %d from %s : %s\n", err, __func__, alpm_strerror(err)); \ + (handle)->pm_errno = (err); \ return (ret); } while(0) #define DOUBLE_EQ(x, y) (fabs((x) - (y)) < DBL_EPSILON) -- 1.7.5.4
On Sat, Jun 18, 2011 at 6:26 PM, Dave Reisner <d@falconindy.com> wrote:
This is an unfortunate chain of events. RET_ERR and RET_ERR_VOID will eventually call CHECK_HANDLE, which resets the handle's pm_errno member. Dan probably had a reason for doing this, so we merely switch the order of operations in the RET_ERR macros to avoid stomping on our pm_errno.
Signed-off-by: Dave Reisner <d@falconindy.com> --- Awesome bug. Here's what happens when the DB is locked:
$ sudo pacman -Sy :: Synchronizing package databases... error: failed to init transaction (unexpected error)
I'm still not sure why CHECK_HANDLE will always reset handle->pm_errno. Seems like it's some sort of lazy initialization that should be getting done elsewhere. It's not so much lazy init, it is required initialization we haven't been doing. The misleading thing may be the CHECK_HANDLE() naming but I couldn't think of anything better that wasn't unwieldy like RESET_ERRNO_AND_ENSURE_HANDLE_EXISTS(). I thought I explained it in my commit message: http://projects.archlinux.org/pacman.git/commit/?id=ee015f086f3c40390659bbc0...
Basically the library was in a position to trip itself up all too easily if a pm_errno value came in and persisted itself through an operation that may need to look at that value.
There may be a better way to solve this, if the reset isn't actually necessary. We did talk about this on IRC but wanted to at least address it here. This does fix it but we are probably better off not calling a public API method to get the callback at all since we are just an internal logger method.
lib/libalpm/util.h | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/lib/libalpm/util.h b/lib/libalpm/util.h index 0549c81..778e20f 100644 --- a/lib/libalpm/util.h +++ b/lib/libalpm/util.h @@ -61,12 +61,14 @@
#define ASSERT(cond, action) do { if(!(cond)) { action; } } while(0)
-#define RET_ERR_VOID(handle, err) do { (handle)->pm_errno = (err); \ +#define RET_ERR_VOID(handle, err) do { \ _alpm_log(handle, PM_LOG_DEBUG, "returning error %d from %s : %s\n", err, __func__, alpm_strerror(err)); \ + (handle)->pm_errno = (err); \ return; } while(0)
-#define RET_ERR(handle, err, ret) do { (handle)->pm_errno = (err); \ +#define RET_ERR(handle, err, ret) do { \ _alpm_log(handle, PM_LOG_DEBUG, "returning error %d from %s : %s\n", err, __func__, alpm_strerror(err)); \ + (handle)->pm_errno = (err); \ return (ret); } while(0)
#define DOUBLE_EQ(x, y) (fabs((x) - (y)) < DBL_EPSILON) -- 1.7.5.4
participants (2)
-
Dan McGee
-
Dave Reisner