diff --git a/c/CHANGELOG.rst b/c/CHANGELOG.rst index 32fd205cef..b2d48a1c03 100644 --- a/c/CHANGELOG.rst +++ b/c/CHANGELOG.rst @@ -8,6 +8,11 @@ nodes of each node in the tree. (:user:`GertjanBisschop`, :issue:`2274`, :pr:`2316`) +**Changes** + +- Reduce the maximum number of rows in a table by 1. This removes edge cases so that a ``tsk_id_t`` can be + used to count the number of rows. (:user:`benjeffery`, :issue:`2336`, :pr:`2337`) + -------------------- [1.0.0] - 2022-05-24 -------------------- diff --git a/c/tests/test_tables.c b/c/tests/test_tables.c index 8da440ed33..aa7280e45f 100644 --- a/c/tests/test_tables.c +++ b/c/tests/test_tables.c @@ -5927,7 +5927,7 @@ test_table_expansion(void) /*Extending by more rows than possible results in overflow*/ ret = tsk_individual_table_extend( - &tables.individuals, &tables2.individuals, TSK_MAX_ID + 1, NULL, 0); + &tables.individuals, &tables2.individuals, TSK_MAX_ID, NULL, 0); CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_TABLE_OVERFLOW); CU_ASSERT_EQUAL_FATAL(tables.individuals.max_rows, 4194305); @@ -5986,7 +5986,7 @@ test_table_expansion(void) CU_ASSERT_EQUAL_FATAL(tables.nodes.max_rows, 4194305); /*Extending by more rows than possible results in overflow*/ - ret = tsk_node_table_extend(&tables.nodes, &tables2.nodes, TSK_MAX_ID + 1, NULL, 0); + ret = tsk_node_table_extend(&tables.nodes, &tables2.nodes, TSK_MAX_ID, NULL, 0); CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_TABLE_OVERFLOW); CU_ASSERT_EQUAL_FATAL(tables.nodes.max_rows, 4194305); @@ -6044,7 +6044,7 @@ test_table_expansion(void) CU_ASSERT_EQUAL_FATAL(tables.edges.max_rows, 4194305); /*Extending by more rows than possible results in overflow*/ - ret = tsk_edge_table_extend(&tables.edges, &tables2.edges, TSK_MAX_ID + 1, NULL, 0); + ret = tsk_edge_table_extend(&tables.edges, &tables2.edges, TSK_MAX_ID, NULL, 0); CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_TABLE_OVERFLOW); CU_ASSERT_EQUAL_FATAL(tables.edges.max_rows, 4194305); @@ -6109,7 +6109,7 @@ test_table_expansion(void) /*Extending by more rows than possible results in overflow*/ ret = tsk_migration_table_extend( - &tables.migrations, &tables2.migrations, TSK_MAX_ID + 1, NULL, 0); + &tables.migrations, &tables2.migrations, TSK_MAX_ID, NULL, 0); CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_TABLE_OVERFLOW); CU_ASSERT_EQUAL_FATAL(tables.migrations.max_rows, 4194305); @@ -6168,7 +6168,7 @@ test_table_expansion(void) CU_ASSERT_EQUAL_FATAL(tables.sites.max_rows, 4194305); /*Extending by more rows than possible results in overflow*/ - ret = tsk_site_table_extend(&tables.sites, &tables2.sites, TSK_MAX_ID + 1, NULL, 0); + ret = tsk_site_table_extend(&tables.sites, &tables2.sites, TSK_MAX_ID, NULL, 0); CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_TABLE_OVERFLOW); CU_ASSERT_EQUAL_FATAL(tables.sites.max_rows, 4194305); @@ -6232,7 +6232,7 @@ test_table_expansion(void) /*Extending by more rows than possible results in overflow*/ ret = tsk_mutation_table_extend( - &tables.mutations, &tables2.mutations, TSK_MAX_ID + 1, NULL, 0); + &tables.mutations, &tables2.mutations, TSK_MAX_ID, NULL, 0); CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_TABLE_OVERFLOW); CU_ASSERT_EQUAL_FATAL(tables.mutations.max_rows, 4194305); @@ -6298,7 +6298,7 @@ test_table_expansion(void) /*Extending by more rows than possible results in overflow*/ ret = tsk_population_table_extend( - &tables.populations, &tables2.populations, TSK_MAX_ID + 1, NULL, 0); + &tables.populations, &tables2.populations, TSK_MAX_ID, NULL, 0); CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_TABLE_OVERFLOW); CU_ASSERT_EQUAL_FATAL(tables.populations.max_rows, 4194305); @@ -6364,7 +6364,7 @@ test_table_expansion(void) /*Extending by more rows than possible results in overflow*/ ret = tsk_provenance_table_extend( - &tables.provenances, &tables2.provenances, TSK_MAX_ID + 1, NULL, 0); + &tables.provenances, &tables2.provenances, TSK_MAX_ID, NULL, 0); CU_ASSERT_EQUAL_FATAL(ret, TSK_ERR_TABLE_OVERFLOW); CU_ASSERT_EQUAL_FATAL(tables.provenances.max_rows, 4194305); @@ -8576,7 +8576,7 @@ test_table_overflow(void) int ret; tsk_id_t ret_id; tsk_table_collection_t tables; - tsk_size_t max_rows = ((tsk_size_t) TSK_MAX_ID) + 1; + tsk_size_t max_rows = ((tsk_size_t) TSK_MAX_ID); ret = tsk_table_collection_init(&tables, 0); CU_ASSERT_EQUAL_FATAL(ret, 0); diff --git a/c/tskit/tables.c b/c/tskit/tables.c index b650b478bc..ee628d24f0 100644 --- a/c/tskit/tables.c +++ b/c/tskit/tables.c @@ -79,13 +79,14 @@ typedef struct { } write_table_ragged_col_t; /* Returns true if adding the specified number of rows would result in overflow. - * Tables can support indexes from 0 to TSK_MAX_ID, and therefore have at most - * TSK_MAX_ID + 1 rows */ + * Tables can support indexes from 0 to TSK_MAX_ID, and therefore could have at most + * TSK_MAX_ID + 1 rows. However we limit to TSK_MAX_ID rows so that counts of rows + * can fit in a tsk_id_t. */ static bool check_table_overflow(tsk_size_t current_size, tsk_size_t additional_rows) { - tsk_size_t max_val = TSK_MAX_ID + (tsk_size_t) 1; - return current_size > (max_val - additional_rows); + tsk_size_t max_val = TSK_MAX_ID; + return additional_rows > max_val || current_size > (max_val - additional_rows); } /* Returns true if adding the specified number of elements would result in overflow @@ -94,7 +95,9 @@ check_table_overflow(tsk_size_t current_size, tsk_size_t additional_rows) static bool check_offset_overflow(tsk_size_t current_size, tsk_size_t additional_elements) { - return current_size > (TSK_MAX_SIZE - additional_elements); + tsk_size_t max_val = TSK_MAX_SIZE; + return additional_elements > max_val + || current_size > (max_val - additional_elements); } #define TSK_NUM_ROWS_UNSET ((tsk_size_t) -1)