Allow database probing if _db_backend is not set #1451
Conversation
|
In case you're wondering where this is coming from: libsolv's rpmdb2solv currently does not work anymore because it does not read in the rpm macros. |
|
I've no problem allowing probing for read-only mode now that it wont go and create new databases in that mode, but I don't think we should ever write based on guessing. Change this to test for read-only dbmode and I'll happily merge. That should work for rpmdb2solv I think... |
|
But is that really a problem? If it detects a known database type, writing should stay with the type anyway, so it's not wrong to also do this for a misconfigured database type. And if it doesn't detect a known database type, it'll use dummydb like before. |
|
Anyway, as it is so simple to test for readonlyness I added the check. |
| @@ -77,7 +78,7 @@ dbDetectBackend(rpmdb rdb) | |||
| } | |||
| } | |||
|
|
|||
| if (!cfg) { | |||
| if (!cfg && ((rdb->db_mode & O_ACCMODE) != O_RDONLY || (rdb->db_flags & RPMDB_FLAG_REBUILD) != 0)) { | |||
There was a problem hiding this comment.
Hmm, does this actually need the REBUILD flag test at all, with the O_RDONLY check in place?
There was a problem hiding this comment.
Maybe not for rpm itself, but I think it should stay to guard against bad librpm users. Otherwise it'll crash with a null pointer deref below in the REBUILD case when trying to print the warning message.
There was a problem hiding this comment.
But that's no longer the case in this latest version, right?
There are quite a few important and subtle checks here so I think we shouldn't mix it up with any redundant ones.
There was a problem hiding this comment.
It will no longer crash, but it's still wrong. IMHO it's better to error out early when opening the old database in rebuilddb mode instead of later when opening the new database.
There is no harm in allowing read access in this case. We still
error out in the database rebuild case, just to be on the safe
side. We now have the following logic:
_db_backend unset:
* error out for rebuilddb or read-write access
* use detected backend and print a debug message
_db_backend unknown:
* error out for rebuilddb or read-write access
* use detected backend and print a warning message
_db_backend set:
* use detected backend and print a warning message if it
does not match the configured backend
|
Ok, updated the pull request. |
|
Thank you! Could you please also cherry pick this into 4.16.1? |
|
Sure - done. |
There is no harm in allowing read access in this case. We still
error out in the database rebuild case, just to be on the safe
side.