Audit: Review ROUTING_FPS_PER_PAGE 4096, and multiple uses of hard-coded 32 in routing_filter.c
Open
Nobody has claimed this yet.
code-cleanup
documentation
- Dominant language
- C
- Stars
- 732
- Forks
- 67
- PR merge metrics
- No merged PRs in 30d
Description
This issue was identified during code-walk thru of PR #230. There are these references in routing_filter.c, which need a better understanding, commenting in the file.
24 #define ROUTING_FPS_PER_PAGE 4096
During discussion, it appeared that 4096 is some sort of "upper-bound" on number of routing filters, but it wasn't clear whether this should be related in any way to page size [etc.]
399 MATRIX_ROWS * MATRIX_COLS + // matrix
400 ROUTING_FPS_PER_PAGE + // fp_buffer
401 ROUTING_FPS_PER_PAGE + // old_fp_buffer
402 ROUTING_FPS_PER_PAGE / 32; // encoding_buffer
413 old_fp_buffer = fp_buffer + ROUTING_FPS_PER_PAGE;
414 encoding_buffer = (uint64 *)(old_fp_buffer + ROUTING_FPS_PER_PAGE);
415 memset(encoding_buffer, 0xff, ROUTING_FPS_PER_PAGE / 32 * sizeof(uint32));
There are many more instances of hard-coded '32' ... and it's not clear what the math is all about here. E.g.
147 routing_get_index(uint32 fp,
148 size_t index_remainder_and_value_size)
149 {
150 return index_remainder_and_value_size == 32
151 ? 0 : fp >> index_remainder_and_value_size;
239 // ffs returns the index + 1 ALEX: I think that's what we want though.
240 bit_offset = __builtin_ffs(encoding_word) - 1;
241 *end = 32 * word + bit_offset;
446 for (uint32 new_fp_no = 0; new_fp_no < num_new_fp; new_fp_no++) {
447 new_fp_arr[new_fp_no] >>= 32 - cfg->fingerprint_size;
448 new_fp_arr[new_fp_no] <<= value_size;
698 platform_assert(cfg->fingerprint_size + value_size <= 32);
811 uint32 fp = hash(slice_data(key), slice_length(key), seed);
812 fp >>= 32 - cfg->fingerprint_size;
And we should find a way to give a semantic #define for 32, and use that consistently where applicable in this file.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Assessment
This issue has not been assessed yet.