Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ByteBuffer uses UInt for Capacity, but Int internally #499

Closed
helje5 opened this issue Jul 3, 2018 · 7 comments
Closed

ByteBuffer uses UInt for Capacity, but Int internally #499

helje5 opened this issue Jul 3, 2018 · 7 comments

Comments

@helje5
Copy link
Contributor

helje5 commented Jul 3, 2018

Separated out of #486, ByteBuffer-core.swift has this:

private static func allocateAndPrepareRawMemory(bytes: Capacity, allocator: Allocator) -> UnsafeMutableRawPointer {
  let bytes = Int(bytes) /* Note(hh): this can fail on 32-bit (Int32 vs UInt32 Capacity)) */

where Capacity is

typealias Capacity = UInt32

This can fail on 32-bit platforms where Int cannot hold any UInt32 value (e.g. 0xDEADBEEF). Apparently the Int is used because of:

let sysMalloc: @convention(c) (Int) -> UnsafeMutableRawPointer? = malloc

TBD: What should be used here. Maybe size_t? That would be better, but theoretically it would still break ByteBuffer on platforms wheresize_t is less than UInt32, e.g. Int32 sounds at least possible for size_t.
Presumably Int is also often used in Swift itself (e.g. the capacity of a UnsafeBufferPointer is probably an Int?)

@weissi
Copy link
Member

weissi commented Jul 4, 2018

ha, that's fun. We chose Int indeed as that's what Swift usually does and indeed UnsafeBufferPointer's count property is of type Int...

@lorentey / @airspeedswift / @moiseev what do you propose?

@lorentey
Copy link
Member

lorentey commented Jul 5, 2018

I think the most pragmatic approach is to limit counts and sizes to values that fit in a signed integer. (>=2GiB buffers are probably unlikely to occur in practice on 32-bit platforms, anyway.)

@weissi
Copy link
Member

weissi commented Jul 5, 2018

I'm happy with saying that ByteBuffer supports 2GB on 32bit and 4GB on 64bit, @Lukasa & @normanmaurer ?

@helje5
Copy link
Contributor Author

helje5 commented Jul 5, 2018

I think a current Raspi comes with 1gb and usually no swap. In other words: I agree, 2gb is enough for everybody.

@Lukasa
Copy link
Contributor

Lukasa commented Jul 5, 2018

No objection from me.

@weissi
Copy link
Member

weissi commented Sep 1, 2018

@helje5 I think we now all agree that 2GB is enough for 32bit so I'll close this, ok?

@helje5
Copy link
Contributor Author

helje5 commented Sep 3, 2018

Sure, fine with me.

@helje5 helje5 closed this as completed Sep 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants