Convert most size_t values to tsk_size_t.

[jeromekelleher]

Clarify the conventions around using size_t vs tsk_size_t

Closes #1573

Removes (nearly) all size_t values from the public API, and clarifies how we should use them within the library. This is a good step towards 1.0 I think, and should clear up some of the mess on 32 bit machines when we have 64 bit tsk_size_t.

[codecov]

Codecov Report

Merging #1579 (4994924) into main (3e427c2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1579   +/-   ##
=======================================
  Coverage   93.60%   93.60%           
=======================================
  Files          27       27           
  Lines       23033    23035    +2     
  Branches     1079     1079           
=======================================
+ Hits        21559    21561    +2     
  Misses       1440     1440           
  Partials       34       34           

Flag	Coverage Δ
c-tests	91.85% <100.00%> (ø)
lwt-tests	92.97% <ø> (ø)
python-c-tests	95.29% <100.00%> (+<0.01%)	⬆️
python-tests	98.80% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
c/tskit/core.h	100.00% <ø> (ø)
c/tskit/genotypes.c	93.89% <ø> (ø)
c/tskit/convert.c	92.04% <100.00%> (ø)
c/tskit/core.c	97.09% <100.00%> (ø)
c/tskit/haplotype_matching.c	95.05% <100.00%> (ø)
c/tskit/tables.c	89.90% <100.00%> (ø)
c/tskit/trees.c	94.86% <100.00%> (ø)
python/_tskitmodule.c	91.84% <100.00%> (+<0.01%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3e427c2...4994924. Read the comment docs.

[benjeffery]

Great!

Searching through for other size_t that should be tsk_size_t these stand out, unless I've misunderstood:

tables.c:7953
tables.c:8230 <-- Seems overflow unsafe, but very unlikely to be an issue?
tables.c:9864

[jeromekelleher]

Thanks @benjeffery!

tables.c:7953

num_sites is being used as the argument to malloc and memcpy, so size_t is correct. I guess there's a potential overflow with the num_sites + 2, but then the size_t would overflow anyway when doing the multiply by sizeof(...), so I don't think there's any problem there.

tables.c:8230 <-- Seems overflow unsafe, but very unlikely to be an issue?

Similarly, num_nodes_alloc is being used as the argument to calloc. Good point about the overflow, let's see. I don't think it's possible, because tables.num_rows is at most 2**31 and size_t will be a uint32_t at least.

There's probably a bunch of tricky overflows around the code to be honest if people are working with large tables on a 32 bit system, because of all the malloc(x * sizeof(...)). But, I doubt one of these large tables would fit into 2G of RAM in the first place anyway, though, so it's probably not worth worrying about.

tables.c:9864
This is correct because we're talking about the size of buffers for the kastore API.

So, I think we're good?

[benjeffery]

Ok, seems my misunderstanding is was parsing "computing memory block sizes" as "counting bytes".