Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rpmtsInitDB() argument confusion #1525

Merged
merged 2 commits into from Feb 2, 2021

Conversation

pmatilai
Copy link
Member

@pmatilai pmatilai commented Feb 2, 2021

Since it's introduction, rpmtsInitDB() has passed the second argument directly to rpmdbInit() as permission bits. However commit 81fef98 incorrectly documented this as being related to the db mode read/write mode, and also used it that way in the python bindings.

While looking at this, noticed that ndb passes hardcoded 0666 to open() whereas the rpm default open mode is 0644, use the API-requested mode instead. Sqlite doesn't seem to support passing a file mode request in the API (but it defaults to 0644 too).

Since it's introduction, rpmtsInitDB() has passed the second argument
directly to rpmdbInit() as permission bits. However commit
81fef98 incorrectly documented this
as being related to the db mode read/write *mode*, and also used it
that way in the python bindings.
Prior to this, ndb files were using hardcoded 0666 permissions whereas
rpm generally defaults to 0644.
@pmatilai pmatilai added the API API related label Feb 2, 2021
@pmatilai
Copy link
Member Author

pmatilai commented Feb 2, 2021

This is all more than just a little hysterical, our own Python bindings have been calling rpmtsInitDB() wrong since forever, presumably only saved by the sanity check for owner writability in newRpmdb(), and the fact that nobody has actually used it because of the implicit database initialization. With that gone, all these little creatures 🐛 🪲 are crawling into the daylight dazed and confused.

We could just as well just make the dbmode argument unused and pass hardcoded 0644 downwards, because this all doesn't really matter that much. Other suggestions also welcome...

Copy link
Member

@Conan-Kudo Conan-Kudo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say we should hardcode 0644 and make the parameter useless.

@pmatilai pmatilai merged commit 77062e6 into rpm-software-management:master Feb 2, 2021
@pmatilai pmatilai deleted the dbinit-pr branch February 2, 2021 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API API related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants