From e269f09438ad1bfaef044c5781615cba45ab7690 Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Thu, 22 Nov 2012 22:23:58 +0100 Subject: Replace localtime() with localtime_r() Since the result of localtime() is stored in a statically allocated structure, data was overwritten when a context switch occurred during (or shortly after) the execution of localtime(), potentially resulting in critical data corruption. BUG#7 and BUG#8 are likely related. This patch converts all usages of localtime() with localtime_r(), which is thread-safe. Reported-by: Baptiste Jonglez Reported-by: Erik Saule Signed-off-by: Lukas Fleischer --- src/apoint.c | 29 ++++++++++----------- src/args.c | 10 ++++---- src/calendar.c | 44 +++++++++++++++---------------- src/day.c | 6 ++--- src/event.c | 13 ++++------ src/io.c | 6 ++--- src/notify.c | 8 +++--- src/pcal.c | 2 +- src/recur.c | 60 +++++++++++++++++++++---------------------- src/utils.c | 81 ++++++++++++++++++++++++++++++++++------------------------ 10 files changed, 133 insertions(+), 126 deletions(-) diff --git a/src/apoint.c b/src/apoint.c index 1483beb..03ccb36 100644 --- a/src/apoint.c +++ b/src/apoint.c @@ -337,39 +337,39 @@ unsigned apoint_inday(struct apoint *i, long start) void apoint_sec2str(struct apoint *o, long day, char *start, char *end) { - struct tm *lt; + struct tm lt; time_t t; if (o->start < day) strncpy(start, "..:..", 6); else { t = o->start; - lt = localtime(&t); - snprintf(start, HRMIN_SIZE, "%02u:%02u", lt->tm_hour, lt->tm_min); + localtime_r(&t, <); + snprintf(start, HRMIN_SIZE, "%02u:%02u", lt.tm_hour, lt.tm_min); } if (o->start + o->dur > day + DAYINSEC) strncpy(end, "..:..", 6); else { t = o->start + o->dur; - lt = localtime(&t); - snprintf(end, HRMIN_SIZE, "%02u:%02u", lt->tm_hour, lt->tm_min); + localtime_r(&t, <); + snprintf(end, HRMIN_SIZE, "%02u:%02u", lt.tm_hour, lt.tm_min); } } void apoint_write(struct apoint *o, FILE * f) { - struct tm *lt; + struct tm lt; time_t t; t = o->start; - lt = localtime(&t); - fprintf(f, "%02u/%02u/%04u @ %02u:%02u", lt->tm_mon + 1, lt->tm_mday, - 1900 + lt->tm_year, lt->tm_hour, lt->tm_min); + localtime_r(&t, <); + fprintf(f, "%02u/%02u/%04u @ %02u:%02u", lt.tm_mon + 1, lt.tm_mday, + 1900 + lt.tm_year, lt.tm_hour, lt.tm_min); t = o->start + o->dur; - lt = localtime(&t); - fprintf(f, " -> %02u/%02u/%04u @ %02u:%02u ", lt->tm_mon + 1, lt->tm_mday, - 1900 + lt->tm_year, lt->tm_hour, lt->tm_min); + localtime_r(&t, <); + fprintf(f, " -> %02u/%02u/%04u @ %02u:%02u ", lt.tm_mon + 1, lt.tm_mday, + 1900 + lt.tm_year, lt.tm_hour, lt.tm_min); if (o->note != NULL) fprintf(f, ">%s ", o->note); @@ -386,10 +386,7 @@ struct apoint *apoint_scan(FILE * f, struct tm start, struct tm end, char state, char *note) { char buf[BUFSIZ], *newline; - time_t tstart, tend, t; - - t = time(NULL); - localtime(&t); + time_t tstart, tend; /* Read the appointment description */ if (!fgets(buf, sizeof buf, f)) diff --git a/src/args.c b/src/args.c index c87a970..01f24e9 100644 --- a/src/args.c +++ b/src/args.c @@ -271,11 +271,11 @@ static void arg_print_date(long date) { char date_str[BUFSIZ]; time_t t; - struct tm *lt; + struct tm lt; t = date; - lt = localtime(&t); - strftime(date_str, BUFSIZ, conf.output_datefmt, lt); + localtime_r(&t, <); + strftime(date_str, BUFSIZ, conf.output_datefmt, <); fputs(date_str, stdout); fputs(":\n", stdout); } @@ -469,7 +469,7 @@ date_arg(const char *ddate, int add_line, const char *fmt_apt, * to format the output correctly. */ timer = time(NULL); - t = *localtime(&timer); + localtime_r(&timer, &t); display_app(&t, numdays, add_line, fmt_apt, fmt_rapt, fmt_ev, fmt_rev, regex); } else { /* a date was entered */ @@ -515,7 +515,7 @@ date_arg_extended(const char *startday, const char *range, int add_line, numdays = atoi(range); } timer = time(NULL); - t = *localtime(&timer); + localtime_r(&timer, &t); if (startday != NULL) { if (parse_date(startday, conf.input_datefmt, (int *)&t.tm_year, (int *)&t.tm_mon, (int *)&t.tm_mday, NULL)) { diff --git a/src/calendar.c b/src/calendar.c index a157caa..48ac84d 100644 --- a/src/calendar.c +++ b/src/calendar.c @@ -140,15 +140,15 @@ void calendar_stop_date_thread(void) void calendar_set_current_date(void) { time_t timer; - struct tm *tm; + struct tm tm; timer = time(NULL); - tm = localtime(&timer); + localtime_r(&timer, &tm); pthread_mutex_lock(&date_thread_mutex); - today.dd = tm->tm_mday; - today.mm = tm->tm_mon + 1; - today.yyyy = tm->tm_year + 1900; + today.dd = tm.tm_mday; + today.mm = tm.tm_mon + 1; + today.yyyy = tm.tm_year + 1900; pthread_mutex_unlock(&date_thread_mutex); } @@ -684,16 +684,16 @@ void calendar_move(enum move move, int count) long calendar_start_of_year(void) { time_t timer; - struct tm *tm; + struct tm tm; timer = time(NULL); - tm = localtime(&timer); - tm->tm_mon = 0; - tm->tm_mday = 1; - tm->tm_hour = 0; - tm->tm_min = 0; - tm->tm_sec = 0; - timer = mktime(tm); + localtime_r(&timer, &tm); + tm.tm_mon = 0; + tm.tm_mday = 1; + tm.tm_hour = 0; + tm.tm_min = 0; + tm.tm_sec = 0; + timer = mktime(&tm); return (long)timer; } @@ -701,17 +701,17 @@ long calendar_start_of_year(void) long calendar_end_of_year(void) { time_t timer; - struct tm *tm; + struct tm tm; timer = time(NULL); - tm = localtime(&timer); - tm->tm_mon = 0; - tm->tm_mday = 1; - tm->tm_hour = 0; - tm->tm_min = 0; - tm->tm_sec = 0; - tm->tm_year++; - timer = mktime(tm); + localtime_r(&timer, &tm); + tm.tm_mon = 0; + tm.tm_mday = 1; + tm.tm_hour = 0; + tm.tm_min = 0; + tm.tm_sec = 0; + tm.tm_year++; + timer = mktime(&tm); return (long)(timer - 1); } diff --git a/src/day.c b/src/day.c index e013df5..29c03e4 100644 --- a/src/day.c +++ b/src/day.c @@ -730,7 +730,7 @@ static void update_rept(struct rpt **rpt, const long start) newuntil = 0; date_entered = 1; } else { - struct tm *lt; + struct tm lt; time_t t; struct date new_date; int newmonth, newday, newyear; @@ -738,11 +738,11 @@ static void update_rept(struct rpt **rpt, const long start) if (parse_date(timstr, conf.input_datefmt, &newyear, &newmonth, &newday, calendar_get_slctd_day())) { t = start; - lt = localtime(&t); + localtime_r(&t, <); new_date.dd = newday; new_date.mm = newmonth; new_date.yyyy = newyear; - newuntil = date2sec(new_date, lt->tm_hour, lt->tm_min); + newuntil = date2sec(new_date, lt.tm_hour, lt.tm_min); if (newuntil < start) { status_mesg(msg_wrong_time, msg_enter); wgetch(win[STA].p); diff --git a/src/event.c b/src/event.c index f460ded..203af44 100644 --- a/src/event.c +++ b/src/event.c @@ -112,13 +112,13 @@ unsigned event_inday(struct event *i, long start) /* Write to file the event in user-friendly format */ void event_write(struct event *o, FILE * f) { - struct tm *lt; + struct tm lt; time_t t; t = o->day; - lt = localtime(&t); - fprintf(f, "%02u/%02u/%04u [%d] ", lt->tm_mon + 1, lt->tm_mday, - 1900 + lt->tm_year, o->id); + localtime_r(&t, <); + fprintf(f, "%02u/%02u/%04u [%d] ", lt.tm_mon + 1, lt.tm_mday, + 1900 + lt.tm_year, o->id); if (o->note != NULL) fprintf(f, ">%s ", o->note); fprintf(f, "%s\n", o->mesg); @@ -128,10 +128,7 @@ void event_write(struct event *o, FILE * f) struct event *event_scan(FILE * f, struct tm start, int id, char *note) { char buf[BUFSIZ], *nl; - time_t tstart, t; - - t = time(NULL); - localtime(&t); + time_t tstart; /* Read the event description */ if (!fgets(buf, sizeof buf, f)) diff --git a/src/io.c b/src/io.c index 9bffe1a..0af8b02 100644 --- a/src/io.c +++ b/src/io.c @@ -454,7 +454,7 @@ void io_load_app(void) { FILE *data_file; int c, is_appointment, is_event, is_recursive; - struct tm start, end, until, *lt; + struct tm start, end, until, lt; llist_t exc; time_t t; int id = 0; @@ -463,8 +463,8 @@ void io_load_app(void) char note[MAX_NOTESIZ + 1], *notep; t = time(NULL); - lt = localtime(&t); - start = end = until = *lt; + localtime_r(&t, <); + start = end = until = lt; data_file = fopen(path_apts, "r"); EXIT_IF(data_file == NULL, _("failed to open appointment file")); diff --git a/src/notify.c b/src/notify.c index a057c2f..10bc0f7 100644 --- a/src/notify.c +++ b/src/notify.c @@ -310,18 +310,18 @@ static void *notify_main_thread(void *arg) const unsigned check_app = MININSEC; int elapse = 0; int got_app; - struct tm *ntime; + struct tm ntime; time_t ntimer; elapse = 0; for (;;) { ntimer = time(NULL); - ntime = localtime(&ntimer); + localtime_r(&ntimer, &ntime); pthread_mutex_lock(¬ify.mutex); pthread_mutex_lock(&nbar.mutex); - strftime(notify.time, NOTIFY_FIELD_LENGTH, nbar.timefmt, ntime); - strftime(notify.date, NOTIFY_FIELD_LENGTH, nbar.datefmt, ntime); + strftime(notify.time, NOTIFY_FIELD_LENGTH, nbar.timefmt, &ntime); + strftime(notify.date, NOTIFY_FIELD_LENGTH, nbar.datefmt, &ntime); pthread_mutex_unlock(&nbar.mutex); pthread_mutex_unlock(¬ify.mutex); notify_update_bar(); diff --git a/src/pcal.c b/src/pcal.c index c4f731d..4c00ddc 100644 --- a/src/pcal.c +++ b/src/pcal.c @@ -64,7 +64,7 @@ foreach_date_dump(const long date_end, struct rpt *rpt, llist_t * exc, time_t t; t = item_first_date; - lt = *localtime(&t); + localtime_r(&t, <); lt.tm_hour = lt.tm_min = lt.tm_sec = 0; lt.tm_isdst = -1; date = mktime(<); diff --git a/src/recur.c b/src/recur.c index e8ddffc..5c32bca 100644 --- a/src/recur.c +++ b/src/recur.c @@ -318,17 +318,17 @@ int recur_char2def(char type) static void recur_write_exc(llist_t * lexc, FILE * f) { llist_item_t *i; - struct tm *lt; + struct tm lt; time_t t; int st_mon, st_day, st_year; LLIST_FOREACH(lexc, i) { struct excp *exc = LLIST_GET_DATA(i); t = exc->st; - lt = localtime(&t); - st_mon = lt->tm_mon + 1; - st_day = lt->tm_mday; - st_year = lt->tm_year + 1900; + localtime_r(&t, <); + st_mon = lt.tm_mon + 1; + st_day = lt.tm_mday; + st_year = lt.tm_year + 1900; fprintf(f, " !%02u/%02u/%04u", st_mon, st_day, st_year); } } @@ -415,27 +415,27 @@ struct recur_event *recur_event_scan(FILE * f, struct tm start, int id, /* Writting of a recursive appointment into file. */ void recur_apoint_write(struct recur_apoint *o, FILE * f) { - struct tm *lt; + struct tm lt; time_t t; t = o->start; - lt = localtime(&t); - fprintf(f, "%02u/%02u/%04u @ %02u:%02u", lt->tm_mon + 1, lt->tm_mday, - 1900 + lt->tm_year, lt->tm_hour, lt->tm_min); + localtime_r(&t, <); + fprintf(f, "%02u/%02u/%04u @ %02u:%02u", lt.tm_mon + 1, lt.tm_mday, + 1900 + lt.tm_year, lt.tm_hour, lt.tm_min); t = o->start + o->dur; - lt = localtime(&t); - fprintf(f, " -> %02u/%02u/%04u @ %02u:%02u", lt->tm_mon + 1, lt->tm_mday, - 1900 + lt->tm_year, lt->tm_hour, lt->tm_min); + localtime_r(&t, <); + fprintf(f, " -> %02u/%02u/%04u @ %02u:%02u", lt.tm_mon + 1, lt.tm_mday, + 1900 + lt.tm_year, lt.tm_hour, lt.tm_min); t = o->rpt->until; if (t == 0) { /* We have an endless recurrent appointment. */ fprintf(f, " {%d%c", o->rpt->freq, recur_def2char(o->rpt->type)); } else { - lt = localtime(&t); + localtime_r(&t, <); fprintf(f, " {%d%c -> %02u/%02u/%04u", o->rpt->freq, - recur_def2char(o->rpt->type), lt->tm_mon + 1, lt->tm_mday, - 1900 + lt->tm_year); + recur_def2char(o->rpt->type), lt.tm_mon + 1, lt.tm_mday, + 1900 + lt.tm_year); } recur_write_exc(&o->exc, f); fputs("} ", f); @@ -451,25 +451,25 @@ void recur_apoint_write(struct recur_apoint *o, FILE * f) /* Writting of a recursive event into file. */ void recur_event_write(struct recur_event *o, FILE * f) { - struct tm *lt; + struct tm lt; time_t t; int st_mon, st_day, st_year; int end_mon, end_day, end_year; t = o->day; - lt = localtime(&t); - st_mon = lt->tm_mon + 1; - st_day = lt->tm_mday; - st_year = lt->tm_year + 1900; + localtime_r(&t, <); + st_mon = lt.tm_mon + 1; + st_day = lt.tm_mday; + st_year = lt.tm_year + 1900; t = o->rpt->until; if (t == 0) { /* We have an endless recurrent event. */ fprintf(f, "%02u/%02u/%04u [%d] {%d%c", st_mon, st_day, st_year, o->id, o->rpt->freq, recur_def2char(o->rpt->type)); } else { - lt = localtime(&t); - end_mon = lt->tm_mon + 1; - end_day = lt->tm_mday; - end_year = lt->tm_year + 1900; + localtime_r(&t, <); + end_mon = lt.tm_mon + 1; + end_day = lt.tm_mday; + end_year = lt.tm_year + 1900; fprintf(f, "%02u/%02u/%04u [%d] {%d%c -> %02u/%02u/%04u", st_mon, st_day, st_year, o->id, o->rpt->freq, recur_def2char(o->rpt->type), end_mon, end_day, end_year); @@ -581,10 +581,10 @@ recur_item_find_occurrence(long item_start, long item_dur, llist_t * item_exc, return 0; t = day_start; - lt_day = *localtime(&t); + localtime_r(&t, <_day); t = item_start; - lt_item = *localtime(&t); + localtime_r(&t, <_item); lt_item_day = lt_item; lt_item_day.tm_sec = lt_item_day.tm_min = lt_item_day.tm_hour = 0; @@ -632,7 +632,7 @@ recur_item_find_occurrence(long item_start, long item_dur, llist_t * item_exc, if (rpt_until != 0 && t > rpt_until) return 0; - lt_item_day = *localtime(&t); + localtime_r(&t, <_item_day); diff = diff_days(lt_item_day, lt_day); if (diff <= span) { @@ -791,7 +791,7 @@ recur_apoint_erase(long start, unsigned num, unsigned delete_whole, */ void recur_repeat_item(void) { - struct tm *lt; + struct tm lt; time_t t; int date_entered = 0; int year = 0, month = 0, day = 0; @@ -876,11 +876,11 @@ void recur_repeat_item(void) if (parse_date(user_input, conf.input_datefmt, &year, &month, &day, calendar_get_slctd_day())) { t = p->start; - lt = localtime(&t); + localtime_r(&t, <); until_date.dd = day; until_date.mm = month; until_date.yyyy = year; - until = date2sec(until_date, lt->tm_hour, lt->tm_min); + until = date2sec(until_date, lt.tm_hour, lt.tm_min); if (until < p->start) { status_mesg(mesg_older, wrong_type_2); wgetch(win[STA].p); diff --git a/src/utils.c b/src/utils.c index 14de867..da8eade 100644 --- a/src/utils.c +++ b/src/utils.c @@ -337,18 +337,26 @@ long get_item_time(long date) int get_item_hour(long date) { - return (localtime((time_t *) & date))->tm_hour; + struct tm lt; + + localtime_r((time_t *)&date, <); + return lt.tm_hour; } int get_item_min(long date) { - return (localtime((time_t *) & date))->tm_min; + struct tm lt; + + localtime_r((time_t *)&date, <); + return lt.tm_min; } long date2sec(struct date day, unsigned hour, unsigned min) { time_t t = now(); - struct tm start = *(localtime(&t)); + struct tm start; + + localtime_r(&t, &start); start.tm_mon = day.mm - 1; start.tm_mday = day.dd; @@ -367,14 +375,14 @@ long date2sec(struct date day, unsigned hour, unsigned min) /* Return a string containing the date, given a date in seconds. */ char *date_sec2date_str(long sec, const char *datefmt) { - struct tm *lt; + struct tm lt; char *datestr = (char *)mem_calloc(BUFSIZ, sizeof(char)); if (sec == 0) strncpy(datestr, "0", BUFSIZ); else { - lt = localtime((time_t *) & sec); - strftime(datestr, BUFSIZ, datefmt, lt); + localtime_r((time_t *)&sec, <); + strftime(datestr, BUFSIZ, datefmt, <); } return datestr; @@ -389,8 +397,9 @@ void date_sec2date_fmt(long sec, const char *fmt, char *datef) setlocale (LC_ALL, "C"); #endif - struct tm *lt = localtime((time_t *)&sec); - strftime(datef, BUFSIZ, fmt, lt); + struct tm lt; + localtime_r((time_t *)&sec, <); + strftime(datef, BUFSIZ, fmt, <); #if ENABLE_NLS setlocale (LC_ALL, locale_old); @@ -403,15 +412,15 @@ void date_sec2date_fmt(long sec, const char *fmt, char *datef) */ long date_sec_change(long date, int delta_month, int delta_day) { - struct tm *lt; + struct tm lt; time_t t; t = date; - lt = localtime(&t); - lt->tm_mon += delta_month; - lt->tm_mday += delta_day; - lt->tm_isdst = -1; - t = mktime(lt); + localtime_r(&t, <); + lt.tm_mon += delta_month; + lt.tm_mday += delta_day; + lt.tm_isdst = -1; + t = mktime(<); EXIT_IF(t == -1, _("failure in mktime")); return t; @@ -423,14 +432,14 @@ long date_sec_change(long date, int delta_month, int delta_day) */ long update_time_in_date(long date, unsigned hr, unsigned mn) { - struct tm *lt; + struct tm lt; time_t t, new_date; t = date; - lt = localtime(&t); - lt->tm_hour = hr; - lt->tm_min = mn; - new_date = mktime(lt); + localtime_r(&t, <); + lt.tm_hour = hr; + lt.tm_min = mn; + new_date = mktime(<); EXIT_IF(new_date == -1, _("error in mktime")); return new_date; @@ -442,7 +451,7 @@ long update_time_in_date(long date, unsigned hr, unsigned mn) */ long get_sec_date(struct date date) { - struct tm *ptrtime; + struct tm ptrtime; time_t timer; long long_date; char current_day[] = "dd "; @@ -451,10 +460,10 @@ long get_sec_date(struct date date) if (date.yyyy == 0 && date.mm == 0 && date.dd == 0) { timer = time(NULL); - ptrtime = localtime(&timer); - strftime(current_day, strlen(current_day), "%d", ptrtime); - strftime(current_month, strlen(current_month), "%m", ptrtime); - strftime(current_year, strlen(current_year), "%Y", ptrtime); + localtime_r(&timer, &ptrtime); + strftime(current_day, strlen(current_day), "%d", &ptrtime); + strftime(current_month, strlen(current_month), "%m", &ptrtime); + strftime(current_year, strlen(current_year), "%Y", &ptrtime); date.mm = atoi(current_month); date.dd = atoi(current_day); date.yyyy = atoi(current_year); @@ -518,16 +527,16 @@ item_in_popup(const char *saved_a_start, const char *saved_a_end, /* Returns the beginning of current day in seconds from 1900. */ long get_today(void) { - struct tm *lt; + struct tm lt; time_t current_time; long current_day; struct date day; current_time = time(NULL); - lt = localtime(¤t_time); - day.mm = lt->tm_mon + 1; - day.dd = lt->tm_mday; - day.yyyy = lt->tm_year + 1900; + localtime_r(¤t_time, <); + day.mm = lt.tm_mon + 1; + day.dd = lt.tm_mday; + day.yyyy = lt.tm_year + 1900; current_day = date2sec(day, 0, 0); return current_day; @@ -541,10 +550,12 @@ long now(void) char *nowstr(void) { + struct tm lt; static char buf[BUFSIZ]; time_t t = now(); - strftime(buf, sizeof buf, "%a %b %d %T %Y", localtime(&t)); + localtime_r(&t, <); + strftime(buf, sizeof buf, "%a %b %d %T %Y", <); return buf; } @@ -1183,15 +1194,17 @@ static void print_date(long date, long day, const char *extformat) printf("%ld", date); else { time_t t = date; - struct tm *lt = localtime((time_t *) & t); + struct tm lt; + + localtime_r((time_t *)&t, <); if (extformat[0] == '\0' || !strcmp(extformat, "default")) { if (date >= day && date <= day + DAYINSEC) - strftime(buf, BUFSIZ, "%H:%M", lt); + strftime(buf, BUFSIZ, "%H:%M", <); else - strftime(buf, BUFSIZ, "..:..", lt); + strftime(buf, BUFSIZ, "..:..", <); } else { - strftime(buf, BUFSIZ, extformat, lt); + strftime(buf, BUFSIZ, extformat, <); } printf("%s", buf); -- cgit v1.2.3 From e16ac0a8a8c18c831e95e1a0799e919e61f5da48 Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Fri, 23 Nov 2012 10:53:37 +0100 Subject: Do not display a mark when files are auto-saved This is kind of useless since users are not able to react to an auto-save notification. It also causes all kinds of problems if the status bar is in a non-standard mode (message, prompt, ...) and is prone to race conditions if the status bar is updated by another thread at the same time. Signed-off-by: Lukas Fleischer --- src/calcurse.h | 1 - src/io.c | 24 ++---------------------- 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/src/calcurse.h b/src/calcurse.h index 7259dd1..e35fe9a 100644 --- a/src/calcurse.h +++ b/src/calcurse.h @@ -580,7 +580,6 @@ enum export_type { /* To customize the display when saving data. */ enum save_display { IO_SAVE_DISPLAY_BAR, - IO_SAVE_DISPLAY_MARK, IO_SAVE_DISPLAY_NONE }; diff --git a/src/io.c b/src/io.c index 0af8b02..0cc394e 100644 --- a/src/io.c +++ b/src/io.c @@ -302,23 +302,6 @@ void io_extract_data(char *dst_data, const char *org, int len) *dst_data = '\0'; } -static void display_mark(void) -{ - const int DISPLAY_TIME = 1; - WINDOW *mwin; - - mwin = newwin(1, 2, 1, col - 3); - - custom_apply_attr(mwin, ATTR_HIGHEST); - mvwaddstr(mwin, 0, 0, "**"); - wins_wrefresh(mwin); - sleep(DISPLAY_TIME); - mvwaddstr(mwin, 0, 0, " "); - wins_wrefresh(mwin); - delwin(mwin); - wins_doupdate(); -} - static pthread_mutex_t io_save_mutex = PTHREAD_MUTEX_INITIALIZER; /* @@ -412,8 +395,6 @@ void io_save_cal(enum save_display display) if (ui_mode == UI_CURSES && display == IO_SAVE_DISPLAY_BAR && conf.progress_bar) show_bar = 1; - else if (ui_mode == UI_CURSES && display == IO_SAVE_DISPLAY_MARK) - display_mark(); if (show_bar) progress_bar(PROGRESS_BAR_SAVE, PROGRESS_BAR_CONF); @@ -436,8 +417,7 @@ void io_save_cal(enum save_display display) ERROR_MSG("%s", access_pb); /* Print a message telling data were saved */ - if (ui_mode == UI_CURSES && conf.system_dialogs - && display != IO_SAVE_DISPLAY_MARK) { + if (ui_mode == UI_CURSES && conf.system_dialogs) { status_mesg(save_success, enter); wgetch(win[STA].p); } @@ -1152,7 +1132,7 @@ static void *io_psave_thread(void *arg) for (;;) { sleep(delay * MININSEC); - io_save_cal(IO_SAVE_DISPLAY_MARK); + io_save_cal(IO_SAVE_DISPLAY_NONE); } } -- cgit v1.2.3 From a80f8dcf2c6eb3b54658218bc081ee9694204dd5 Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Fri, 23 Nov 2012 09:59:06 +0100 Subject: Lock screen when drawing on the calendar/notification panel Lock the screen if either the calendar panel or the notification bar is updated to avoid race conditions. Addresses BUG#6. Note that we currently always use a screen-level lock, even if only one window is affected. This is to be changed in the future. Signed-off-by: Lukas Fleischer --- src/calcurse.h | 4 ++++ src/calendar.c | 26 +++++++++++++++++++++++--- src/notify.c | 6 ++++++ src/wins.c | 28 ++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/calcurse.h b/src/calcurse.h index e35fe9a..7a7f4f2 100644 --- a/src/calcurse.h +++ b/src/calcurse.h @@ -975,6 +975,10 @@ void vars_init(void); /* wins.c */ extern struct window win[NBWINS]; +unsigned wins_nbar_lock(void); +void wins_nbar_unlock(void); +unsigned wins_calendar_lock(void); +void wins_calendar_unlock(void); int wins_refresh(void); int wins_wrefresh(WINDOW *); int wins_doupdate(void); diff --git a/src/calendar.c b/src/calendar.c index 48ac84d..42ee449 100644 --- a/src/calendar.c +++ b/src/calendar.c @@ -297,6 +297,7 @@ draw_monthly_view(struct window *cwin, struct date *current_day, c_day_1 = (int)((ymd_to_scalar(yr, mo, 1 + sunday_first) - (long)1) % 7L); /* Write the current month and year on top of the calendar */ + wins_calendar_lock(); custom_apply_attr(cwin->p, ATTR_HIGHEST); mvwprintw(cwin->p, ofs_y, (SBAR_WIDTH - (strlen(_(monthnames[mo - 1])) + 5)) / 2, @@ -310,6 +311,7 @@ draw_monthly_view(struct window *cwin, struct date *current_day, mvwaddstr(cwin->p, ofs_y, ofs_x + 4 * j, _(daynames[1 + j - sunday_first])); } custom_remove_attr(cwin->p, ATTR_HIGHEST); + wins_calendar_unlock(); day_1_sav = (c_day_1 + 1) * 3 + c_day_1 - 7; @@ -327,11 +329,12 @@ draw_monthly_view(struct window *cwin, struct date *current_day, ofs_x = OFFX - day_1_sav - 4 * c_day; } - /* This is today, so print it in yellow. */ + wins_calendar_lock(); if (c_day == current_day->dd && current_day->mm == slctd_day.mm && current_day->yyyy == slctd_day.yyyy && current_day->dd != slctd_day.dd) { + /* This is today, so print it in yellow. */ custom_apply_attr(cwin->p, ATTR_LOWEST); mvwprintw(cwin->p, ofs_y + 1, ofs_x + day_1_sav + 4 * c_day + 1, "%2d", c_day); @@ -347,10 +350,12 @@ draw_monthly_view(struct window *cwin, struct date *current_day, mvwprintw(cwin->p, ofs_y + 1, ofs_x + day_1_sav + 4 * c_day + 1, "%2d", c_day); custom_remove_attr(cwin->p, ATTR_LOW); - } else + } else { /* otherwise, print normal days in black */ mvwprintw(cwin->p, ofs_y + 1, ofs_x + day_1_sav + 4 * c_day + 1, "%2d", c_day); + } + wins_calendar_unlock(); } } @@ -454,9 +459,11 @@ draw_weekly_view(struct window *cwin, struct date *current_day, /* Print the week number. */ weeknum = ISO8601weeknum(&t); + wins_calendar_lock(); custom_apply_attr(cwin->p, ATTR_HIGHEST); mvwprintw(cwin->p, 2, cwin->w - 9, "(# %02d)", weeknum); custom_remove_attr(cwin->p, ATTR_HIGHEST); + wins_calendar_unlock(); /* Now draw calendar view. */ for (j = 0; j < WEEKINDAYS; j++) { @@ -488,22 +495,28 @@ draw_weekly_view(struct window *cwin, struct date *current_day, else attr = 0; + wins_calendar_lock(); if (attr) custom_apply_attr(cwin->p, attr); mvwprintw(cwin->p, OFFY + 1, OFFX + 1 + 4 * j, "%02d", t.tm_mday); if (attr) custom_remove_attr(cwin->p, attr); + wins_calendar_unlock(); /* Draw slices indicating appointment times. */ memset(slices, 0, DAYSLICESNO * sizeof *slices); if (day_chk_busy_slices(date, DAYSLICESNO, slices)) { for (i = 0; i < DAYSLICESNO; i++) { - if (j != WEEKINDAYS - 1 && i != DAYSLICESNO - 1) + if (j != WEEKINDAYS - 1 && i != DAYSLICESNO - 1) { + wins_calendar_lock(); mvwhline(cwin->p, OFFY + 2 + i, OFFX + 3 + 4 * j, ACS_S9, 2); + wins_calendar_unlock(); + } if (slices[i]) { int highlight; highlight = (t.tm_mday == slctd_day.dd) ? 1 : 0; + wins_calendar_lock(); if (highlight) custom_apply_attr(cwin->p, attr); wattron(cwin->p, A_REVERSE); @@ -512,6 +525,7 @@ draw_weekly_view(struct window *cwin, struct date *current_day, wattroff(cwin->p, A_REVERSE); if (highlight) custom_remove_attr(cwin->p, attr); + wins_calendar_unlock(); } } } @@ -521,11 +535,13 @@ draw_weekly_view(struct window *cwin, struct date *current_day, } /* Draw marks to indicate midday on the sides of the calendar. */ + wins_calendar_lock(); custom_apply_attr(cwin->p, ATTR_HIGHEST); mvwhline(cwin->p, OFFY + 1 + DAYSLICESNO / 2, OFFX, ACS_S9, 1); mvwhline(cwin->p, OFFY + 1 + DAYSLICESNO / 2, OFFX + WCALWIDTH - 3, ACS_S9, 1); custom_remove_attr(cwin->p, ATTR_HIGHEST); + wins_calendar_unlock(); #undef DAYSLICESNO } @@ -537,8 +553,12 @@ void calendar_update_panel(struct window *cwin) unsigned sunday_first; calendar_store_current_date(¤t_day); + + wins_calendar_lock(); erase_window_part(cwin->p, 1, 3, cwin->w - 2, cwin->h - 2); mvwhline(cwin->p, 2, 1, ACS_HLINE, cwin->w - 2); + wins_calendar_unlock(); + sunday_first = calendar_week_begins_on_monday()? 0 : 1; draw_calendar[calendar_view] (cwin, ¤t_day, sunday_first); diff --git a/src/notify.c b/src/notify.c index 10bc0f7..6ac94b9 100644 --- a/src/notify.c +++ b/src/notify.c @@ -240,11 +240,13 @@ void notify_update_bar(void) app_pos = file_pos + strlen(notify.apts_file) + 2 + space; txt_max_len = col - (app_pos + 12 + space); + wins_nbar_lock(); custom_apply_attr(notify.win, ATTR_HIGHEST); wattron(notify.win, A_UNDERLINE | A_REVERSE); mvwhline(notify.win, 0, 0, ACS_HLINE, col); mvwprintw(notify.win, 0, date_pos, "[ %s | %s ]", notify.date, notify.time); mvwprintw(notify.win, 0, file_pos, "(%s)", notify.apts_file); + wins_nbar_unlock(); pthread_mutex_lock(¬ify_app.mutex); if (notify_app.got_app) { @@ -271,6 +273,7 @@ void notify_update_bar(void) else blinking = 0; + wins_nbar_lock(); if (blinking) wattron(notify.win, A_BLINK); if (too_long) @@ -281,6 +284,7 @@ void notify_update_bar(void) hours_left, minutes_left, notify_app.txt); if (blinking) wattroff(notify.win, A_BLINK); + wins_nbar_unlock(); if (blinking) notify_launch_cmd(); @@ -295,8 +299,10 @@ void notify_update_bar(void) } pthread_mutex_unlock(¬ify_app.mutex); + wins_nbar_lock(); wattroff(notify.win, A_UNDERLINE | A_REVERSE); custom_remove_attr(notify.win, ATTR_HIGHEST); + wins_nbar_unlock(); wins_wrefresh(notify.win); pthread_mutex_unlock(¬ify.mutex); diff --git a/src/wins.c b/src/wins.c index d518377..64dd962 100644 --- a/src/wins.c +++ b/src/wins.c @@ -76,6 +76,32 @@ static void screen_release(void) pthread_mutex_unlock(&screen_mutex); } +/* + * FIXME: The following functions currently lock the whole screen. Use both + * window-level and screen-level mutexes (or use use_screen() and use_window(), + * see curs_threads(3)) to avoid locking too much. + */ + +unsigned wins_nbar_lock(void) +{ + return screen_acquire(); +} + +void wins_nbar_unlock(void) +{ + screen_release(); +} + +unsigned wins_calendar_lock(void) +{ + return screen_acquire(); +} + +void wins_calendar_unlock(void) +{ + screen_release(); +} + int wins_refresh(void) { int rc; @@ -476,10 +502,12 @@ static void border_nocolor(WINDOW * window) void wins_update_border(int flags) { if (flags & FLAG_CAL) { + wins_calendar_lock(); if (slctd_win == CAL) border_color(win[CAL].p); else border_nocolor(win[CAL].p); + wins_calendar_unlock(); } if (flags & FLAG_APP) { if (slctd_win == APP) -- cgit v1.2.3 From 0ea23c24bf06e153bb075804e195e1733fd67d3f Mon Sep 17 00:00:00 2001 From: Lukas Fleischer Date: Fri, 23 Nov 2012 10:41:20 +0100 Subject: Release screen mutex if thread dies We did not setup a thread cleanup procedure which resulted in calcurse freezing if a thread tried to draw on the screen after another thread was canceled while locking the screen. Note that this kind of cleanup handlers should be added to other mutexes as well. This patch just removes the most common case of triggering a deadlock. Also note that we cannot move pthread_cleanup_push() and pthread_cleanup_pop() into the locking/unlocking functions since both pthread_cleanup_push() and pthread_cleanup_pop() may be implemented as macros that must be used in pairs within the same lexical scope. Signed-off-by: Lukas Fleischer --- src/calcurse.h | 18 ++++++++++++++++++ src/calendar.c | 32 ++++++++++++++++---------------- src/notify.c | 12 ++++++------ src/wins.c | 42 ++++++++++++++++++++++++++++++++---------- 4 files changed, 72 insertions(+), 32 deletions(-) diff --git a/src/calcurse.h b/src/calcurse.h index 7a7f4f2..d7b5093 100644 --- a/src/calcurse.h +++ b/src/calcurse.h @@ -456,6 +456,22 @@ enum win { #define FLAG_STA (1 << STA) #define FLAG_ALL ((1 << NBWINS) - 1) +#define WINS_NBAR_LOCK \ + pthread_cleanup_push(wins_nbar_cleanup, NULL); \ + wins_nbar_lock(); + +#define WINS_NBAR_UNLOCK \ + wins_nbar_unlock(); \ + pthread_cleanup_pop(0); + +#define WINS_CALENDAR_LOCK \ + pthread_cleanup_push(wins_calendar_cleanup, NULL); \ + wins_calendar_lock(); + +#define WINS_CALENDAR_UNLOCK \ + wins_calendar_unlock(); \ + pthread_cleanup_pop(0); + enum ui_mode { UI_CURSES, UI_CMDLINE, @@ -977,8 +993,10 @@ void vars_init(void); extern struct window win[NBWINS]; unsigned wins_nbar_lock(void); void wins_nbar_unlock(void); +void wins_nbar_cleanup(void *); unsigned wins_calendar_lock(void); void wins_calendar_unlock(void); +void wins_calendar_cleanup(void *); int wins_refresh(void); int wins_wrefresh(WINDOW *); int wins_doupdate(void); diff --git a/src/calendar.c b/src/calendar.c index 42ee449..df247df 100644 --- a/src/calendar.c +++ b/src/calendar.c @@ -297,7 +297,7 @@ draw_monthly_view(struct window *cwin, struct date *current_day, c_day_1 = (int)((ymd_to_scalar(yr, mo, 1 + sunday_first) - (long)1) % 7L); /* Write the current month and year on top of the calendar */ - wins_calendar_lock(); + WINS_CALENDAR_LOCK; custom_apply_attr(cwin->p, ATTR_HIGHEST); mvwprintw(cwin->p, ofs_y, (SBAR_WIDTH - (strlen(_(monthnames[mo - 1])) + 5)) / 2, @@ -311,7 +311,7 @@ draw_monthly_view(struct window *cwin, struct date *current_day, mvwaddstr(cwin->p, ofs_y, ofs_x + 4 * j, _(daynames[1 + j - sunday_first])); } custom_remove_attr(cwin->p, ATTR_HIGHEST); - wins_calendar_unlock(); + WINS_CALENDAR_UNLOCK; day_1_sav = (c_day_1 + 1) * 3 + c_day_1 - 7; @@ -329,7 +329,7 @@ draw_monthly_view(struct window *cwin, struct date *current_day, ofs_x = OFFX - day_1_sav - 4 * c_day; } - wins_calendar_lock(); + WINS_CALENDAR_LOCK; if (c_day == current_day->dd && current_day->mm == slctd_day.mm && current_day->yyyy == slctd_day.yyyy @@ -355,7 +355,7 @@ draw_monthly_view(struct window *cwin, struct date *current_day, mvwprintw(cwin->p, ofs_y + 1, ofs_x + day_1_sav + 4 * c_day + 1, "%2d", c_day); } - wins_calendar_unlock(); + WINS_CALENDAR_UNLOCK; } } @@ -459,11 +459,11 @@ draw_weekly_view(struct window *cwin, struct date *current_day, /* Print the week number. */ weeknum = ISO8601weeknum(&t); - wins_calendar_lock(); + WINS_CALENDAR_LOCK; custom_apply_attr(cwin->p, ATTR_HIGHEST); mvwprintw(cwin->p, 2, cwin->w - 9, "(# %02d)", weeknum); custom_remove_attr(cwin->p, ATTR_HIGHEST); - wins_calendar_unlock(); + WINS_CALENDAR_UNLOCK; /* Now draw calendar view. */ for (j = 0; j < WEEKINDAYS; j++) { @@ -495,28 +495,28 @@ draw_weekly_view(struct window *cwin, struct date *current_day, else attr = 0; - wins_calendar_lock(); + WINS_CALENDAR_LOCK; if (attr) custom_apply_attr(cwin->p, attr); mvwprintw(cwin->p, OFFY + 1, OFFX + 1 + 4 * j, "%02d", t.tm_mday); if (attr) custom_remove_attr(cwin->p, attr); - wins_calendar_unlock(); + WINS_CALENDAR_UNLOCK; /* Draw slices indicating appointment times. */ memset(slices, 0, DAYSLICESNO * sizeof *slices); if (day_chk_busy_slices(date, DAYSLICESNO, slices)) { for (i = 0; i < DAYSLICESNO; i++) { if (j != WEEKINDAYS - 1 && i != DAYSLICESNO - 1) { - wins_calendar_lock(); + WINS_CALENDAR_LOCK; mvwhline(cwin->p, OFFY + 2 + i, OFFX + 3 + 4 * j, ACS_S9, 2); - wins_calendar_unlock(); + WINS_CALENDAR_UNLOCK; } if (slices[i]) { int highlight; highlight = (t.tm_mday == slctd_day.dd) ? 1 : 0; - wins_calendar_lock(); + WINS_CALENDAR_LOCK; if (highlight) custom_apply_attr(cwin->p, attr); wattron(cwin->p, A_REVERSE); @@ -525,7 +525,7 @@ draw_weekly_view(struct window *cwin, struct date *current_day, wattroff(cwin->p, A_REVERSE); if (highlight) custom_remove_attr(cwin->p, attr); - wins_calendar_unlock(); + WINS_CALENDAR_UNLOCK; } } } @@ -535,13 +535,13 @@ draw_weekly_view(struct window *cwin, struct date *current_day, } /* Draw marks to indicate midday on the sides of the calendar. */ - wins_calendar_lock(); + WINS_CALENDAR_LOCK; custom_apply_attr(cwin->p, ATTR_HIGHEST); mvwhline(cwin->p, OFFY + 1 + DAYSLICESNO / 2, OFFX, ACS_S9, 1); mvwhline(cwin->p, OFFY + 1 + DAYSLICESNO / 2, OFFX + WCALWIDTH - 3, ACS_S9, 1); custom_remove_attr(cwin->p, ATTR_HIGHEST); - wins_calendar_unlock(); + WINS_CALENDAR_UNLOCK; #undef DAYSLICESNO } @@ -554,10 +554,10 @@ void calendar_update_panel(struct window *cwin) calendar_store_current_date(¤t_day); - wins_calendar_lock(); + WINS_CALENDAR_LOCK; erase_window_part(cwin->p, 1, 3, cwin->w - 2, cwin->h - 2); mvwhline(cwin->p, 2, 1, ACS_HLINE, cwin->w - 2); - wins_calendar_unlock(); + WINS_CALENDAR_UNLOCK; sunday_first = calendar_week_begins_on_monday()? 0 : 1; diff --git a/src/notify.c b/src/notify.c index 6ac94b9..ebc9728 100644 --- a/src/notify.c +++ b/src/notify.c @@ -240,13 +240,13 @@ void notify_update_bar(void) app_pos = file_pos + strlen(notify.apts_file) + 2 + space; txt_max_len = col - (app_pos + 12 + space); - wins_nbar_lock(); + WINS_NBAR_LOCK; custom_apply_attr(notify.win, ATTR_HIGHEST); wattron(notify.win, A_UNDERLINE | A_REVERSE); mvwhline(notify.win, 0, 0, ACS_HLINE, col); mvwprintw(notify.win, 0, date_pos, "[ %s | %s ]", notify.date, notify.time); mvwprintw(notify.win, 0, file_pos, "(%s)", notify.apts_file); - wins_nbar_unlock(); + WINS_NBAR_UNLOCK; pthread_mutex_lock(¬ify_app.mutex); if (notify_app.got_app) { @@ -273,7 +273,7 @@ void notify_update_bar(void) else blinking = 0; - wins_nbar_lock(); + WINS_NBAR_LOCK; if (blinking) wattron(notify.win, A_BLINK); if (too_long) @@ -284,7 +284,7 @@ void notify_update_bar(void) hours_left, minutes_left, notify_app.txt); if (blinking) wattroff(notify.win, A_BLINK); - wins_nbar_unlock(); + WINS_NBAR_UNLOCK; if (blinking) notify_launch_cmd(); @@ -299,10 +299,10 @@ void notify_update_bar(void) } pthread_mutex_unlock(¬ify_app.mutex); - wins_nbar_lock(); + WINS_NBAR_LOCK; wattroff(notify.win, A_UNDERLINE | A_REVERSE); custom_remove_attr(notify.win, ATTR_HIGHEST); - wins_nbar_unlock(); + WINS_NBAR_UNLOCK; wins_wrefresh(notify.win); pthread_mutex_unlock(¬ify.mutex); diff --git a/src/wins.c b/src/wins.c index 64dd962..249610c 100644 --- a/src/wins.c +++ b/src/wins.c @@ -40,6 +40,14 @@ #include "calcurse.h" +#define SCREEN_ACQUIRE \ + pthread_cleanup_push(screen_cleanup, (void *)NULL); \ + screen_acquire(); + +#define SCREEN_RELEASE \ + screen_release(); \ + pthread_cleanup_pop(0); + /* Variables to handle calcurse windows. */ struct window win[NBWINS]; @@ -76,6 +84,11 @@ static void screen_release(void) pthread_mutex_unlock(&screen_mutex); } +static void screen_cleanup(void *arg) +{ + screen_release(); +} + /* * FIXME: The following functions currently lock the whole screen. Use both * window-level and screen-level mutexes (or use use_screen() and use_window(), @@ -92,6 +105,11 @@ void wins_nbar_unlock(void) screen_release(); } +void wins_nbar_cleanup(void *arg) +{ + wins_nbar_unlock(); +} + unsigned wins_calendar_lock(void) { return screen_acquire(); @@ -102,14 +120,18 @@ void wins_calendar_unlock(void) screen_release(); } +void wins_calendar_cleanup(void *arg) +{ + wins_calendar_unlock(); +} + int wins_refresh(void) { int rc; - if (!screen_acquire()) - return ERR; + SCREEN_ACQUIRE; rc = refresh(); - screen_release(); + SCREEN_RELEASE; return rc; } @@ -118,10 +140,11 @@ int wins_wrefresh(WINDOW * win) { int rc; - if (!win || !screen_acquire()) + if (!win) return ERR; + SCREEN_ACQUIRE; rc = wrefresh(win); - screen_release(); + SCREEN_RELEASE; return rc; } @@ -130,10 +153,9 @@ int wins_doupdate(void) { int rc; - if (!screen_acquire()) - return ERR; + SCREEN_ACQUIRE; rc = doupdate(); - screen_release(); + SCREEN_RELEASE; return rc; } @@ -502,12 +524,12 @@ static void border_nocolor(WINDOW * window) void wins_update_border(int flags) { if (flags & FLAG_CAL) { - wins_calendar_lock(); + WINS_CALENDAR_LOCK; if (slctd_win == CAL) border_color(win[CAL].p); else border_nocolor(win[CAL].p); - wins_calendar_unlock(); + WINS_CALENDAR_UNLOCK; } if (flags & FLAG_APP) { if (slctd_win == APP) -- cgit v1.2.3