[pacman-dev] [PATCH] Remove need to explicitly register the local DB
There is rarely a reason to ever not set up the local DB when getting a
libalpm handle, so just do it automatically at this time. It is still a lazy
initialization anyway, so there should be little to no fallout from doing
it this way.
Signed-off-by: Dan McGee
Dan McGee wrote:
There is rarely a reason to ever not set up the local DB when getting a libalpm handle, so just do it automatically at this time. It is still a lazy initialization anyway, so there should be little to no fallout from doing it this way.
I'm concerned about the uncertainty of "little to no fallout". What exactly does the "lazy" initialization do? Does it in any way rely on the presence of a local database? I ask because I have a tool in mind that will use ALPM to query the sync database, most likely in the absence of a local database. It also strikes me as bad design to hard-code something that is not necessary for all operations, especially for a library that is at least intended for use beyond Pacman. Wouldn't it make more sense to create a separate routine to automatically set up the local DB when getting a libalpm handle? Applications that need the local DB could just use that instead wherever they acquire the handle.
On 29/01/11 14:51, Xyne wrote:
Dan McGee wrote:
There is rarely a reason to ever not set up the local DB when getting a libalpm handle, so just do it automatically at this time. It is still a lazy initialization anyway, so there should be little to no fallout from doing it this way.
I'm concerned about the uncertainty of "little to no fallout". What exactly does the "lazy" initialization do? Does it in any way rely on the presence of a local database?
The lazy initialisation means that nothing is actually loaded when registering the local database. The local database is only read in as needed and only the parts that are needed are read in. In fact, all that is done registering the local database is creating a pmdb_t srtuct and assigning the local database operation struct to it. That is the "little to no fallout" if you do not use the local database. Allan
Allan McRae wrote:
The lazy initialisation means that nothing is actually loaded when registering the local database. The local database is only read in as needed and only the parts that are needed are read in.
In fact, all that is done registering the local database is creating a pmdb_t srtuct and assigning the local database operation struct to it. That is the "little to no fallout" if you do not use the local database.
Allan
Thanks for the clarification. Even if it is negligible, it's munging separate logic together. Given that the purpose is presumably to save coders a few lines of code, I still think it would make more sense to use a wrapper function to do that when getting the handle. Don't worry though, I'll leave it there as I have no intention of pestering you about something that is effectively irrelevant. I just see it as lazy design in a bad way. Regards, Xyne
On Sat, Jan 29, 2011 at 3:09 AM, Xyne
Allan McRae wrote:
The lazy initialisation means that nothing is actually loaded when registering the local database. The local database is only read in as needed and only the parts that are needed are read in.
In fact, all that is done registering the local database is creating a pmdb_t srtuct and assigning the local database operation struct to it. That is the "little to no fallout" if you do not use the local database.
Allan
Thanks for the clarification.
Even if it is negligible, it's munging separate logic together. Given that the purpose is presumably to save coders a few lines of code, I still think it would make more sense to use a wrapper function to do that when getting the handle.
The "register" code could just as well be done in the handle setup where I moved the call anyway. You'd save about 13 CPU cycles. It's design that is meant to remove stupidity in a library where you have to make what seems like a no-op call- but make sure you only do it once. Why let users shoot themselves in the foot? We already handled the teardown in alpm_release so this just makes the initialize side map that. -Dan -Dan
On Sat, Jan 29, 2011 at 9:59 AM, Dan McGee
-Dan
-Dan
^^ A sign I need my morning coffee... :)
On Sat, Jan 29, 2011 at 10:09 AM, Xyne
Even if it is negligible, it's munging separate logic together. Given that the purpose is presumably to save coders a few lines of code, I still think it would make more sense to use a wrapper function to do that when getting the handle.
It's not separate logic. Dan commit log might have been a bit misleading. alpm_initialize initializes (wow) whatever is needed for the other functions to work. What we do there is just allocate some structures like the handle, and now handle->db_local as well. We are talking about one calloc and one strdup here to setup db_local. Really no big deal, there is no loading of the db, not even a check that one exists at all. And it does make it simpler for libalpm users. And as Dan said its symetric with alpm_release.
Dan McGee wrote:
The "register" code could just as well be done in the handle setup where I moved the call anyway. You'd save about 13 CPU cycles. It's design that is meant to remove stupidity in a library where you have to make what seems like a no-op call- but make sure you only do it once. Why let users shoot themselves in the foot? We already handled the teardown in alpm_release so this just makes the initialize side map that.
Xavier Chantry wrote:
It's not separate logic. Dan commit log might have been a bit misleading.
alpm_initialize initializes (wow) whatever is needed for the other functions to work. What we do there is just allocate some structures like the handle, and now handle->db_local as well. We are talking about one calloc and one strdup here to setup db_local. Really no big deal, there is no loading of the db, not even a check that one exists at all. And it does make it simpler for libalpm users. And as Dan said its symetric with alpm_release.
Ah. It makes much more sense now, especially given the symmetry with alpm_release. Thanks.
-Dan
-Dan
^^ A sign I need my morning coffee... :)
I just assumed that you had graduated to Double Dan: https://secure.wikimedia.org/wikipedia/en/wiki/Dan_%28rank%29
participants (5)
-
Allan McRae
-
Dan McGee
-
Dan McGee
-
Xavier Chantry
-
Xyne