FS#23732 - [libalpm] can't commit empty transactions (alpm_trans_prepare rejects them w/o error code)

Attached to Project: Pacman
Opened by Rémy Oudompheng (remyoudompheng) - Tuesday, 12 April 2011, 22:46 GMT
Task Type Bug Report
Category Backend/Core
Status Unconfirmed
Assigned To No-one
Architecture All
Severity Low
Priority Normal
Reported Version 3.5.1
Due in Version Undecided
Due Date Undecided
Percent Complete 0%
Votes 0
Private No

Details

Summary and Info:
Comments in the code suggest that the following behaviour is a feature: alpm_trans_prepare() will silently exit without setting any error when receiving an empty transaction. *However* it will not mark the transaction as prepared.

This triggers an error when trying to commit the transaction.

Suggested possible changes:
* mark empty transactions as prepared
* return -1 and set errno to say "empty transactions are invalid"

Steps to Reproduce:
* write naïve code like this:

alpm_trans_init(0, NULL, NULL, NULL);
/* do nothing */
int ret = alpm_trans_prepare(NULL)
if (ret != 0) return;
alpm_trans_commit(NULL)
alpm_trans_release();
return;
This task depends upon

Comment by Dan McGee (toofishes) - Tuesday, 12 April 2011, 23:07 GMT
Can you link to said comments in cgit showing where you think the behavior is a feature, please? I'm not sure about those, because this sounds like a bug.
Comment by Rémy Oudompheng (remyoudompheng) - Wednesday, 13 April 2011, 05:19 GMT
http://projects.archlinux.org/pacman.git/tree/lib/libalpm/trans.c#n199

Line 199 in trans.c (on branch "master") says "If there's nothing to do, return without complaining", so I believe it was somewhat intended in the first place.
Comment by Dan McGee (toofishes) - Wednesday, 13 April 2011, 15:37 GMT
d04baaba lib/libalpm/trans.c (Judd Vinet 2005-03-15 01:51:43 +0000 250) /* If there's nothing to do, return without complaining */
8ff3b870 lib/libalpm/trans.c (Xavier Chantry 2009-07-15 19:14:01 +0200 251) if(trans->add == NULL && trans->remove == NULL) {
0303b26b lib/libalpm/trans.c (Dan McGee 2011-03-20 19:45:57 -0500 252) return 0;
04e054f3 lib/libalpm/trans.c (Aurelien Foret 2005-04-06 18:25:33 +0000 253) }

You're looking at a quite old comment; I'd say fix the behavior as you see appropriate in this case. It looks like trans_commit() has the same little sanity check, so one one also need to advance the stage there as well if you were to go with your first suggestion.
Comment by Dan McGee (toofishes) - Wednesday, 13 April 2011, 15:38 GMT
Sidenote: the blame output in the previous comment came from `git blame -MBCw`, which should follow lines back to their source no matter how many times they have moved around.

Loading...