Hi,
attached is a tricky patch: It skip database rewrites to disk when no changes actually happened. The current issue is that there is no locking/notify mechanism for the database file and, especially if using autosave=true, multiple abook instances imply that "latest closed wins". Reproducer: - term1 run abook - term2 run mutt, add an email (trigger abook --add-email) # here datafile is changed on disk with the new email - close abook in term1 # here datafile is changed on disk, the email is lost There are several ways to avoid/warn when this happens, but all of them first needs that we avoid file rewrites when no change happened, that means isolating write operations. Currently: $ ls -i .abook/addressbook 8535 .abook/addressbook $ abook # press "q" to quit $ ls -i .abook/addressbook 7943 .abook/addressbook This is what the patch attempts to solve by marking the database "need save" after (every) "write" operation on items. That allowed adding a "*" indicator à la emacs to show the "db modified" status. The more sensible here is to ensure that *every* possible action implying modification of the database will really triggers db_need_save = TRUE. If some are missing from the patch the related modifications will *not* be saved to disk. That is to say that this patch has to be *sure*. I need to think more about the behavior when autosave=false but IHMO the check for "is db modified" must be activated for all *saving* vectors, that means that pressing "w" to explicitly save the database to disk will *not* rewrite if there is no modification. I tried many (if not all) of the possible operations offered by the UI and hope to not have missed any. But as shown by the hunk affecting edit.c, database write operations are not strictly restricted to database.c. Further testing welcome. Next I'd see a more robust way to handle concurrent write accesses to datafile. I can think about file-lock and inotify and I'd rather want to avoid any need of datafile "merge". regards -- GPG id: 0xD7F62B21
---
database.c | 22 ++++++++++++++++++++++
edit.c | 2 ++
ui.c | 8 ++++++--
3 files changed, 30 insertions(+), 2 deletions(-)
diff --git a/database.c b/database.c
index 7dd4a9a..bea31fd 100644
--- a/database.c
+++ b/database.c
@@ -62,6 +62,7 @@ abook_field standard_fields[] = {
{0} /* ITEM_FIELDS */
};
+bool db_need_save = FALSE;
extern int first_list_item;
extern int curitem;
@@ -294,6 +295,8 @@ next:
xfree(line);
item_free(&item);
+ db_need_save = FALSE;
+
return 0;
}
@@ -350,6 +353,9 @@ write_database(FILE *out, struct db_enumerator e)
int
save_database()
{
+
+ if(! db_need_save) return 0;
+
FILE *out;
int ret = 0;
struct db_enumerator e = init_db_enumerator(ENUM_ALL);
@@ -377,6 +383,11 @@ save_database()
if((rename(datafile_new, datafile)) == -1)
ret = -1;
+ if(ret == 0) {
+ db_need_save = FALSE;
+ refresh_screen();
+ }
+
out:
free(datafile_new);
free(datafile_old);
@@ -486,6 +497,7 @@ add_item2database(list_item item)
database[LAST_ITEM] = item_create();
item_copy(database[LAST_ITEM], item);
+ db_need_save = TRUE;
return 0;
}
@@ -511,6 +523,7 @@ remove_selected_items()
}
item_free(&database[LAST_ITEM]);
items--;
+ db_need_save = TRUE;
}
}
@@ -546,6 +559,7 @@ void merge_selected_items()
}
item_free(&database[LAST_ITEM]);
items--;
+ db_need_save = TRUE;
}
}
@@ -577,6 +591,7 @@ void remove_duplicates()
}
item_free(&database[LAST_ITEM]);
items--;
+ db_need_save = TRUE;
}
}
@@ -656,6 +671,7 @@ sort_by_field(char *name)
sort_field = field;
qsort((void *)database, items, sizeof(list_item), namecmp);
+ db_need_save = TRUE;
refresh_screen();
}
@@ -666,6 +682,7 @@ sort_surname()
select_none();
qsort((void *)database, items, sizeof(list_item), surnamecmp);
+ db_need_save = TRUE;
refresh_screen();
}
@@ -805,6 +822,9 @@ void
item_copy(list_item dest, list_item src)
{
memmove(dest, src, ITEM_SIZE);
+ // call by parse_database(), that's why
+ // parse_database() re-init db_need_save to FALSE
+ db_need_save = TRUE;
}
void
@@ -873,6 +893,7 @@ item_fput(list_item item, int i, char *val)
if(id != -1) {
item[id] = val;
+ db_need_save = TRUE;
return 1;
}
@@ -901,6 +922,7 @@ real_db_field_put(int item, int i, int std, char *val)
if(id != -1) {
database[item][id] = val;
+ db_need_save = TRUE;
return 1;
}
diff --git a/edit.c b/edit.c
index 9a76c8a..9ad468c 100644
--- a/edit.c
+++ b/edit.c
@@ -37,6 +37,7 @@ static void locale_date(char *str, size_t str_len, int year, int month, int day)
*/
extern int views_count;
+extern bool db_need_save;
WINDOW *editw;
@@ -304,6 +305,7 @@ change_field(char *msg, char **field, size_t max_len)
*field = ui_readline(msg, old, max_len - 1, 0);
if(*field) {
+ db_need_save = TRUE;
xfree(old);
if(!**field)
xfree(*field);
diff --git a/ui.c b/ui.c
index e879885..8d8ace0 100644
--- a/ui.c
+++ b/ui.c
@@ -44,6 +44,8 @@ extern char *datafile;
extern bool alternative_datafile;
+extern bool db_need_save;
+
/*
* internal variables
*/
@@ -726,8 +728,10 @@ ui_find(int next)
void
ui_print_number_of_items()
{
- char *str = strdup_printf(" " "|%3d/%3d",
- selected_items(), db_n_items());
+ char *str = strdup_printf(" %c" "|%3d/%3d",
+ db_need_save ? '*' : ' ',
+ selected_items(),
+ db_n_items());
attrset(COLOR_PAIR(CP_HEADER));
mvaddstr(0, COLS-strlen(str), str);
--
1.7.10.4
signature.asc
Description: Digital signature
------------------------------------------------------------------------------ Is your legacy SCM system holding you back? Join Perforce May 7 to find out: • 3 signs your SCM is hindering your productivity • Requirements for releasing software faster • Expert tips and advice for migrating your SCM now http://p.sf.net/sfu/perforce
_______________________________________________ Abook-devel mailing list Abook-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/abook-devel
