[pacman-dev] trans.c : reworking of transaction interruptions.

[Xavier]

On Tue, Sep 11, 2007 at 02:18:40PM +0200, Xavier wrote:
> On Sat, Sep 08, 2007 at 12:45:14AM +0200, Xavier wrote:
> > diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> > index f62e588..2db4346 100644
> > --- a/src/pacman/pacman.c
> > +++ b/src/pacman/pacman.c
> > @@ -208,8 +208,8 @@ static void cleanup(int signum)
> >  		pm_fprintf(stderr, PM_LOG_ERROR, "Internal pacman error: Segmentation fault.\n"
> >  		        "Please submit a full bug report with --debug if appropriate.\n");
> >  		exit(signum);
> > -	} else if((signum == SIGINT) && (alpm_trans_release() == -1)
> > -						&& (pm_errno == PM_ERR_TRANS_COMMITING)) {
> > +	} else if((signum == SIGINT)) {
> > +		alpm_trans_interrupt();
> >  		return;
> >  	}
> >  
> 
> hmm, my brain wasn't correctly working when I did this, I guess.
> I didn't like this part, but instead of rewriting it, I just removed it
> without trying to understand...
> 
> The previous code only caught ctrl+C when pacman was commiting a trans,
> and only in this case it didn't exit directly. In all other cases, it exited.
> With my patch, it never exits.
> 
> I actually wonder if that doesn't invalid my whole patch, I am not sure
> yet and have to think more about it.
> 
> Dan, please don't pull this yet, and sorry.

I see the main problem is to detect if a transaction is being commited or
not.
If yes, we just return from that cleanup function, and we wait for the
transaction to end.
If no, we can release the transaction and exit pacman.

The code previously used a return code of -1 from alpm_trans_release +
pm_errno for detecting that.
I thought about using a return code of 0 from alpm_trans_interrupt for
replacing it.
I don't know if it makes the code more clear or more obscure though...



>From a7c9f19e86900d128aa6722094a0abc7978fc29e Mon Sep 17 00:00:00 2001
From: Chantry Xavier <shiningxc at gmail.com>
Date: Wed, 12 Sep 2007 19:09:55 +0200
Subject: [PATCH] Handle SIGINT correctly in all cases.

Signed-off-by: Chantry Xavier <shiningxc at gmail.com>
---
 lib/libalpm/trans.c |    3 ++-
 src/pacman/pacman.c |    9 +++++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/lib/libalpm/trans.c b/lib/libalpm/trans.c
index dbc6038..9eb27c3 100644
--- a/lib/libalpm/trans.c
+++ b/lib/libalpm/trans.c
@@ -179,7 +179,8 @@ int SYMEXPORT alpm_trans_interrupt()
 
 	trans = handle->trans;
 	ASSERT(trans != NULL, RET_ERR(PM_ERR_TRANS_NULL, -1));
-	ASSERT(trans->state != STATE_IDLE, RET_ERR(PM_ERR_TRANS_NULL, -1));
+	ASSERT(trans->state == STATE_COMMITING || trans->state == STATE_INTERRUPTED,
+			RET_ERR(PM_ERR_TRANS_TYPE, -1));
 
 	trans->state = STATE_INTERRUPTED;
 
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
index 2db4346..540025b 100644
--- a/src/pacman/pacman.c
+++ b/src/pacman/pacman.c
@@ -209,8 +209,13 @@ static void cleanup(int signum)
 		        "Please submit a full bug report with --debug if appropriate.\n");
 		exit(signum);
 	} else if((signum == SIGINT)) {
-		alpm_trans_interrupt();
-		return;
+		if(alpm_trans_interrupt() == 0) {
+			/* a transaction is being interrupted, don't exit pacman yet. */
+			return;
+		} else {
+			/* no commiting transaction, we can release it now and then exit pacman */
+			alpm_trans_release();
+		}
 	}
 
 	/* free alpm library resources */
-- 
1.5.3