[PATCH] Refactor: split off emptying a package's file info

Here's the first refactoring patch mentioned in "dpkg list-file performance".

===
Refactor: split off emptying a package's file info

Put it into a separate function for reuse by other routines and to
simplify ensure_packagefiles_available.

Signed-off-by: David Benjamin <davidben.TakeThisOut@mit.edu>
---
src/filesdb.c | 38 +++++++++++++++++++++++++-------------
1 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/src/filesdb.c b/src/filesdb.c
index da8cde2..a437a40 100644
--- a/src/filesdb.c
+++ b/src/filesdb.c
@@ -69,22 +69,15 @@ void note_must_reread_files_inpackage(struct pkginfo *pkg) {

static int saidread=0;

- /* load the list of files in this package into memory, or update the
- * list if it is there but stale
+ /* erase the files saved in pkg
*/
-void ensure_packagefiles_available(struct pkginfo *pkg) {
- int fd;
- const char *filelistfile;
- struct fileinlist **lendp, *newent, *current;
+static void _erase_pkg_file_data(struct pkginfo *pkg) {
+ struct fileinlist *current;
struct filepackages *packageslump;
- int search, findlast, putat;
- struct stat stat_buf;
- char *loaded_list, *loaded_list_end, *thisline, *nextline, *ptr;
+ int search, findlast;

- if (pkg->clientdata && pkg->clientdata->fileslistvalid) return;
- ensure_package_clientdata(pkg);
-
- /* Throw away any the stale data, if there was any. */
+ if (!pkg->clientdata)
+ return; /* nothing to empty */
for (current= pkg->clientdata->files;
current;
current= current->next) {
@@ -121,6 +114,25 @@ void ensure_packagefiles_available(struct pkginfo *pkg) {
*/
}
pkg->clientdata->files = NULL;
+}
+
+ /* load the list of files in this package into memory, or update the
+ * list if it is there but stale
+ */
+void ensure_packagefiles_available(struct pkginfo *pkg) {
+ int fd;
+ const char *filelistfile;
+ struct fileinlist **lendp, *newent;
+ struct filepackages *packageslump;
+ int putat;
+ struct stat stat_buf;
+ char *loaded_list, *loaded_list_end, *thisline, *nextline, *ptr;
+
+ if (pkg->clientdata && pkg->clientdata->fileslistvalid) return;
+ ensure_package_clientdata(pkg);
+
+ /* Throw away any the stale data, if there was any. */
+ _erase_pkg_file_data(pkg);

/* Packages which aren't installed don't have a files list. */
if (pkg->status == stat_notinstalled) {
--
1.6.3.3


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST.TakeThisOut@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster.TakeThisOut@lists.debian.org

Hi!

Some comments on the changes I made when applying it.

On Sat, 2009-08-29 at 19:57:24 -0400, David Benjamin wrote:
> Here's the first refactoring patch mentioned in "dpkg list-file performance".
>
> ===
> Refactor: split off emptying a package's file info
>
> Put it into a separate function for reuse by other routines and to
> simplify ensure_packagefiles_available.
>
> Signed-off-by: David Benjamin <davidben.RemoveThis@mit.edu>

Fixed the commit log. If you need to add out of band information in
the future, you can append it after the three dashes, which git
ignores.

> ---
> src/filesdb.c | 38 +++++++++++++++++++++++++-------------
> 1 files changed, 25 insertions(+), 13 deletions(-)
>
> diff --git a/src/filesdb.c b/src/filesdb.c
> index da8cde2..a437a40 100644
> --- a/src/filesdb.c
> +++ b/src/filesdb.c
> @@ -69,22 +69,15 @@ void note_must_reread_files_inpackage(struct pkginfo *pkg) {
> static int saidread=0;
> - /* load the list of files in this package into memory, or update the
> - * list if it is there but stale
> + /* erase the files saved in pkg
> */
> -void ensure_packagefiles_available(struct pkginfo *pkg) {
> - int fd;
> - const char *filelistfile;
> - struct fileinlist **lendp, *newent, *current;
> +static void _erase_pkg_file_data(struct pkginfo *pkg) {

No underscores at the beginning or end of identifiers. Formatted the
changed code and comment to the new coding style by placing the
function name to column 0 and the brace on the next line. Renamed the
function to pkg_files_blank.

> + struct fileinlist *current;
> struct filepackages *packageslump;
> - int search, findlast, putat;
> - struct stat stat_buf;
> - char *loaded_list, *loaded_list_end, *thisline, *nextline, *ptr;
> + int search, findlast;
> - if (pkg->clientdata && pkg->clientdata->fileslistvalid) return;
> - ensure_package_clientdata(pkg);
> -
> - /* Throw away any the stale data, if there was any. */
> + if (!pkg->clientdata)
> + return; /* nothing to empty */

Added a blank line here.

> for (current= pkg->clientdata->files;
> current;
> current= current->next) {
> @@ -121,6 +114,25 @@ void ensure_packagefiles_available(struct pkginfo *pkg) {
> */
> }
> pkg->clientdata->files = NULL;
> +}
> +
> + /* load the list of files in this package into memory, or update the
> + * list if it is there but stale
> + */

Reformatted this comment as well.

> +void ensure_packagefiles_available(struct pkginfo *pkg) {

Same format change as the other function.

> + int fd;
> + const char *filelistfile;
> + struct fileinlist **lendp, *newent;
> + struct filepackages *packageslump;
> + int putat;
> + struct stat stat_buf;
> + char *loaded_list, *loaded_list_end, *thisline, *nextline, *ptr;
> +
> + if (pkg->clientdata && pkg->clientdata->fileslistvalid) return;

Placed return on the next line.

> + ensure_package_clientdata(pkg);
> +
> + /* Throw away any the stale data, if there was any. */
> + _erase_pkg_file_data(pkg);

Added a blank line here.

> /* Packages which aren't installed don't have a files list. */
> if (pkg->status == stat_notinstalled) {

Pushed, thanks!

regards,
guillem


--
To UNSUBSCRIBE, email to debian-dpkg-REQUEST.RemoveThis@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster.RemoveThis@lists.debian.org