diff mbox

[pacman-dev,v2] Do not continuously try to open an invalid database

Message ID 20180109120713.19532-1-allan@archlinux.org
State Superseded, archived
Headers show

Commit Message

Allan McRae Jan. 9, 2018, 12:07 p.m. UTC
If you manage to download a bad database (e.g. an html file when
behind a proxy or with a badly configured webserver), pacman makes
sure you know about it.  Here is some example output:

error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format

I don't know how many times that gets printed because it goes beyond my scrollback
buffer.

Flag a database that we can "open" and "fstat" but not read from as invalid to avoid
this.

Signed-off-by: Allan McRae <allan@archlinux.org>
---

Review of v1:
https://lists.archlinux.org/pipermail/pacman-dev/2017-May/022027.html

v2 - do much less stuff to achive the same result...


 lib/libalpm/be_sync.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Dave Reisner Jan. 9, 2018, 1:58 p.m. UTC | #1
On Tue, Jan 09, 2018 at 10:07:13PM +1000, Allan McRae wrote:
> If you manage to download a bad database (e.g. an html file when
> behind a proxy or with a badly configured webserver), pacman makes
> sure you know about it.  Here is some example output:
> 
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
> 
> I don't know how many times that gets printed because it goes beyond my scrollback
> buffer.
> 
> Flag a database that we can "open" and "fstat" but not read from as invalid to avoid
> this.
> 
> Signed-off-by: Allan McRae <allan@archlinux.org>
> ---
> 
> Review of v1:
> https://lists.archlinux.org/pipermail/pacman-dev/2017-May/022027.html
> 
> v2 - do much less stuff to achive the same result...
> 
> 
>  lib/libalpm/be_sync.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
> index 1b7c8b6f..389e964c 100644
> --- a/lib/libalpm/be_sync.c
> +++ b/lib/libalpm/be_sync.c
> @@ -486,6 +486,7 @@ static int sync_db_populate(alpm_db_t *db)
>  	fd = _alpm_open_archive(db->handle, dbpath, &buf,
>  			&archive, ALPM_ERR_DB_OPEN);
>  	if(fd < 0) {
> +		db->status &= DB_STATUS_INVALID;

Given that DB_STATUS_INVALID is a single bit, shouldn't this be a simple
assignment? A bitwise 'and' here will result in the status remaining
invalid if it's already invalid, or zero'ing it out (DB_STATUS_VALID) if
it's some other set of bits. That doesn't seem like what we want.

Semi-related thought: it's a bit odd that we semantically chose "0" as
valid, and "1" as invalid.

>  		return -1;
>  	}
>  	est_count = estimate_package_count(&buf, archive);
> -- 
> 2.15.1
Geert Hendrickx via pacman-dev Jan. 9, 2018, 9:04 p.m. UTC | #2
>-------- Original Message --------
>Subject: Re: [pacman-dev] [PATCH v2] Do not continuously try to open an invalid database
>Local Time: January 9, 2018 2:58 PM
>UTC Time: January 9, 2018 1:58 PM
>From: d@falconindy.com
>To: Discussion list for pacman development <pacman-dev@archlinux.org>
>
>On Tue, Jan 09, 2018 at 10:07:13PM +1000, Allan McRae wrote:
>>If you manage to download a bad database (e.g. an html file when
>> behind a proxy or with a badly configured webserver), pacman makes
>> sure you know about it.  Here is some example output:
>>error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>> error: could not open file /var/lib/pacman/sync/extra.db: Unrecognized archive format
>>I don't know how many times that gets printed because it goes beyond my scrollback
>> buffer.
>>Flag a database that we can "open" and "fstat" but not read from as invalid to avoid
>> this.
>>Signed-off-by: Allan McRae allan@archlinux.org
>>
>>Review of v1:
>>https://lists.archlinux.org/pipermail/pacman-dev/2017-May/022027.html
>>v2 - do much less stuff to achive the same result...
>>lib/libalpm/be_sync.c | 1 +
>> 1 file changed, 1 insertion(+)
>>diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
>> index 1b7c8b6f..389e964c 100644
>> --- a/lib/libalpm/be_sync.c
>> +++ b/lib/libalpm/be_sync.c
>> @@ -486,6 +486,7 @@ static int sync_db_populate(alpm_db_t *db)
>> fd = _alpm_open_archive(db->handle, dbpath, &buf,
>> &archive, ALPM_ERR_DB_OPEN);
>> if(fd < 0) {
>> - db->status &= DB_STATUS_INVALID;
>>
>>
> Given that DB_STATUS_INVALID is a single bit, shouldn't this be a simple
> assignment? A bitwise 'and' here will result in the status remaining
> invalid if it's already invalid, or zero'ing it out (DB_STATUS_VALID) if
> it's some other set of bits. That doesn't seem like what we want.
>


He's right. &= SOMETHING seems odd. There are three usual operations for single bit assignments: &= ~SOMETHING, |= SOMETHING and ^= SOMETHING.

cheers!
mar77i

‚Äč
Sent with ProtonMail Secure Email.
diff mbox

Patch

diff --git a/lib/libalpm/be_sync.c b/lib/libalpm/be_sync.c
index 1b7c8b6f..389e964c 100644
--- a/lib/libalpm/be_sync.c
+++ b/lib/libalpm/be_sync.c
@@ -486,6 +486,7 @@  static int sync_db_populate(alpm_db_t *db)
 	fd = _alpm_open_archive(db->handle, dbpath, &buf,
 			&archive, ALPM_ERR_DB_OPEN);
 	if(fd < 0) {
+		db->status &= DB_STATUS_INVALID;
 		return -1;
 	}
 	est_count = estimate_package_count(&buf, archive);