GNOME Bugzilla – Bug 77878
Bug in newsrc.c, mark_range_unread
Last modified: 2004-12-22 21:47:04 UTC
This procedure marks range of readed messages as unread. This is a test, can be compiled anywhere in the newsrc.c: Newsrc N; Range R; N.group_low = 1; N.group_high = 10; N.read = g_array_new( FALSE, FALSE, sizeof(Range)); R.low = 1; R.high = 4; g_array_append_val (N.read, R); R.low = 7; R.high = 9; g_array_append_val (N.read, R); / * Now the ranges are 1-4, 7-9 R.low = 3; R.high = 4; mark_range_unread ( &N, &R); /* Now the ranges should be 1-2, 7-9 but are 1-2, 5-4, 7-9 !! */ The procedure `mark_range_unread' checks if deleted interval is inside another and in this case splits the another interval, but the deleted interval can be also on the *edge*. Here is a patch: diff -cr pan-0.11.2.90-bug/pan/base/newsrc.c pan-0.11.2.90/pan/base/newsrc.c *** pan-0.11.2.90-bug/pan/base/newsrc.c Mon Dec 3 21:42:13 2001 --- pan-0.11.2.90/pan/base/newsrc.c Sun Apr 7 00:58:06 2002 *************** *** 327,342 **** g_array_remove_index (a, i); --high_index; } ! else if (range_contains (r, ur)) /* split */ { ! Range range; ! range.high = r->high; ! r->high = ur->low-1; ! range.low = ur->high+1; ! retval += ur->high+1-ur->low; ! g_array_insert_val (a, low_index+1, range); ! ++high_index; ! i += 2; } else if (range_contains_point (r, ur->low)) /* change high */ { --- 327,355 ---- g_array_remove_index (a, i); --high_index; } ! else if (range_contains (r, ur)) /* split or cut */ { ! if ( r->low == ur->low ) /* cut low */ ! { ! r->low = ur->high+1; ! retval += ur->high+1-ur->low; ! } ! else if ( r->high == ur->high ) /* cut high */ ! { ! r->high = ur->low-1; ! retval += ur->high+1-ur->low; ! } ! else ! { ! Range range; /* Split */ ! range.high = r->high; ! r->high = ur->low-1; ! range.low = ur->high+1; ! retval += ur->high+1-ur->low; ! g_array_insert_val (a, low_index+1, range); ! ++high_index; ! i += 2; ! } } else if (range_contains_point (r, ur->low)) /* change high */ { Here is a patch for newsrc.c, which checks the ranges on both ends of procedures `mark_range_unread' and `mark_range_read' and prints complete information if something is incorrect. I have used it for debugging: diff -cr pan-0.11.2.90-dbg/pan/base/newsrc.c pan-0.11.2.90/pan/base/newsrc.c *** pan-0.11.2.90-dbg/pan/base/newsrc.c Sun Apr 7 01:26:52 2002 --- pan-0.11.2.90/pan/base/newsrc.c Sun Apr 7 01:20:25 2002 *************** *** 216,227 **** --- 216,263 ---- return merged; } + int check_range(Newsrc *n, const char *message) + { + Range *r1, *r2; guint i; + if (n->read->len < 2 ) return 0; + for ( i=1; i< n->read->len; i++ ) { + r1 = &g_array_index (n->read, Range, i-1); + r2 = &g_array_index (n->read, Range, i ); + if ( r1->low> r1->high) { + printf("%s r1.low>r1.high: %lu-%lu\n", message, r1->low, r1->high); + return 1; + } + if ( r2->low > r2->high) { + printf("%s r2.low>r2.high: %lu-%lu\n", message, r2->low, r2->high); + return 1; + } + if ( r1->high >= r2->low) { + printf("%s r1.high >= r2.low: %lu-%lu\n", message, r1->low, r2->high); + return 1; + } + } + return 0; + } + + static void print_range(const GArray* rr) + { + guint i; Range *r; + printf("*** Begin\n"); + for ( i=0; i < rr->len; i++ ) { + r = &g_array_index (rr, Range, i); + printf( "%lu-%lu ", r->low, r->high); + } + printf( "*** End\n"); + } + /** * @return the number of articles newly-marked as read */ static gulong mark_range_read (Newsrc * n, Range * rr) { + GArray *ranges_before = g_array_new( FALSE, FALSE, sizeof(Range)); + GArray * a = n->read; gint i; gint low_index = 0; *************** *** 229,234 **** --- 265,273 ---- gulong retval = 0; gboolean range_found = FALSE; + g_array_append_vals( ranges_before, n->read->data, n->read->len); + check_range( n, "mark_read - before" ); + low_index = lower_bound (&rr->low, a->data, a->len, *************** *** 290,301 **** --- 329,350 ---- maybe_merge_ranges (a, low_index-1); } + if (check_range( n, "mark_read - after" )) { + printf( "Inserted range: %lu-%lu", rr->low, rr->high); + printf( "Error in check range\nRanges before:\n"); + print_range(ranges_before); + printf( "Ranges after\n"); + print_range(n->read); + } + g_array_free(ranges_before, FALSE); return retval; } static gulong mark_range_unread (Newsrc * n, const Range * ur) { + GArray *ranges_before = g_array_new( FALSE, FALSE, sizeof(Range)); + gint i; gint low_index = -1; gint high_index = -1; *************** *** 304,309 **** --- 353,361 ---- gulong retval = 0; GArray * a = n->read; + g_array_append_vals( ranges_before, n->read->data, n->read->len); + check_range( n, "mark_unread - before" ); + low_index = lower_bound (&ur->low, a->data, a->len, *************** *** 366,371 **** --- 418,431 ---- else ++i; } + if (check_range( n, "mark_unread - after" )) { + printf( "Removed range: %lu-%lu", ur->low, ur->high); + printf( "Error in check range\nRanges before:\n"); + print_range(ranges_before); + printf( "Ranges after\n"); + print_range(n->read); + } + g_array_free(ranges_before, FALSE); return retval; } Jiri Kubicek
Great work tracking this down and making a patch! I've got a lot of regression tests on newsrc, but somehow this border condition never got noticed til now. Thanks _very_ much. This is fixed in CVS now.
btw, check out pan/tests/test-newsrc.c to see how I've been testing these ranges so far. I think maybe the debugging utils you worked up here should be made into a wrapper in test-newsrc.c that wraps mark_range().