Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

LUCY-295 BitVector size_t ticks#40

Merged
asfgit merged 6 commits intoapache:masterfrom
rectang:LUCY-295-bitvec-size
Apr 13, 2016
Merged

LUCY-295 BitVector size_t ticks#40
asfgit merged 6 commits intoapache:masterfrom
rectang:LUCY-295-bitvec-size

Conversation

@rectang
Copy link
Contributor

@rectang rectang commented Apr 13, 2016

Change BitVector to use size_t instead of uint32_t to address bits in the array.

rectang added 6 commits April 12, 2016 21:38
Have BitVector use a size_t to track its capacity and to index into the
bit array.
Create a dedicated static function which given a bit size returns a byte
size.
BitVector API functions now take size_t indexes rather than uint32_t.
Changing indexes from uint32_t to size_t requires updating a fair amount
of test code.
BitVec counts and sizes are now measured in size_t not uint32_t.
@nwellnhof
Copy link
Contributor

Next_Hit and To_Array still work with int32_t indices. For now, we should at least add a check like

if (ivars->cap > INT32_MAX) { THROW(...) }

In the long run, I'd suggest to change the return value of Next_Hit, remove To_Array, and switch its users over to Next_Hit.

return SIZE_MAX / 8;
}
return (bit_size + 7) / 8;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overflow check should be:

if (bit_size > SIZE_MAX - 7) {
    return SIZE_MAX / 8 + 1;
}

Or maybe an alternative implementation:

static CFISH_INLINE size_t
SI_octet_size(size_t bit_size) {
    if (bit_size == 0) { return 0; }
    return (bit_size - 1) / 8 + 1;
}

@asfgit asfgit merged commit 42640a6 into apache:master Apr 13, 2016
asfgit pushed a commit that referenced this pull request Apr 13, 2016
Change BitVector to use `size_t` instead of `uint32_t` to address
bits in the array.

This fixes #40.
@rectang rectang deleted the LUCY-295-bitvec-size branch April 15, 2016 00:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants