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
", 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
", 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=ee015f086f3c40390659bbc0129b7c08ffd0ed5f

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
", 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
", 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
>
>
>