Skip to content

Commit

Permalink
Use CInt and CUnsignedInt when calling C APIs (#128)
Browse files Browse the repository at this point in the history
Motivation:

On different platforms C may define `int` and `unsigned` to be different
types. When writing C interop code in Swift it's preferable to
use the `CInt` and `CUnsignedInt` typealiases as they will resolve to
the correct C type on all supported platforms. If the imported C API is
a definite-sized type, like `uint32_t` we leave the Swift type as
definitely-sized.

Modifications:

Use `CInt` instead of `Int32` for C `int` parameters and `CUnsignedInt`
instead of `UInt32` for C `unsigned` parameters.

Result:

Code is more portable and definite-sized types stand out.
  • Loading branch information
kevints authored and weissi committed Aug 12, 2019
1 parent 833617d commit c8ac6c1
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 25 deletions.
8 changes: 4 additions & 4 deletions Sources/NIOSSL/SSLCertificate.swift
Expand Up @@ -89,7 +89,7 @@ public class NIOSSLCertificate {
/// DER format.
public convenience init(bytes: [UInt8], format: NIOSSLSerializationFormats) throws {
let ref = bytes.withUnsafeBytes { (ptr) -> UnsafeMutablePointer<X509>? in
let bio = CNIOBoringSSL_BIO_new_mem_buf(UnsafeMutableRawPointer(mutating: ptr.baseAddress!), Int32(ptr.count))!
let bio = CNIOBoringSSL_BIO_new_mem_buf(UnsafeMutableRawPointer(mutating: ptr.baseAddress!), CInt(ptr.count))!

defer {
CNIOBoringSSL_BIO_free(bio)
Expand Down Expand Up @@ -145,8 +145,8 @@ public class NIOSSLCertificate {

// Per the man page, to find the first entry we set lastIndex to -1. When there are no
// more entries, -1 is returned as the index of the next entry.
var lastIndex: Int32 = -1
var nextIndex: Int32 = -1
var lastIndex: CInt = -1
var nextIndex: CInt = -1
repeat {
lastIndex = nextIndex
nextIndex = CNIOBoringSSL_X509_NAME_get_index_by_NID(subjectName, NID_commonName, lastIndex)
Expand Down Expand Up @@ -220,7 +220,7 @@ extension NIOSSLCertificate {
}

return try bytes.withUnsafeBytes { (ptr) -> [NIOSSLCertificate] in
let bio = CNIOBoringSSL_BIO_new_mem_buf(UnsafeMutableRawPointer(mutating: ptr.baseAddress!), Int32(ptr.count))!
let bio = CNIOBoringSSL_BIO_new_mem_buf(UnsafeMutableRawPointer(mutating: ptr.baseAddress!), CInt(ptr.count))!
defer {
CNIOBoringSSL_BIO_free(bio)
}
Expand Down
18 changes: 9 additions & 9 deletions Sources/NIOSSL/SSLConnection.swift
Expand Up @@ -193,7 +193,7 @@ internal final class SSLConnection {
/// `getDataForNetwork` after this method is called. This method also consumes
/// data from internal buffers: call `consumeDataFromNetwork` before calling this
/// method.
func doHandshake() -> AsyncOperationResult<Int32> {
func doHandshake() -> AsyncOperationResult<CInt> {
CNIOBoringSSL_ERR_clear_error()
let rc = CNIOBoringSSL_SSL_do_handshake(ssl)

Expand All @@ -219,7 +219,7 @@ internal final class SSLConnection {
/// `getDataForNetwork` after this method is called. This method also consumes
/// data from internal buffers: call `consumeDataFromNetwork` before calling this
/// method.
func doShutdown() -> AsyncOperationResult<Int32> {
func doShutdown() -> AsyncOperationResult<CInt> {
CNIOBoringSSL_ERR_clear_error()
let rc = CNIOBoringSSL_SSL_shutdown(ssl)

Expand Down Expand Up @@ -275,16 +275,16 @@ internal final class SSLConnection {
// safely return any of the error values that SSL_read might provide here because writeWithUnsafeMutableBytes
// will try to use that as the number of bytes written and blow up. If we could prevent it doing that (which
// we can with reading) that would be grand, but we can't, so instead we need to use a temp variable. Not ideal.
var bytesRead: Int32 = 0
var bytesRead: CInt = 0
let rc = outputBuffer.writeWithUnsafeMutableBytes { (pointer) -> Int in
bytesRead = CNIOBoringSSL_SSL_read(self.ssl, pointer.baseAddress, Int32(pointer.count))
bytesRead = CNIOBoringSSL_SSL_read(self.ssl, pointer.baseAddress, CInt(pointer.count))
return bytesRead >= 0 ? Int(bytesRead) : 0
}

if bytesRead > 0 {
return .complete(rc)
} else {
let result = CNIOBoringSSL_SSL_get_error(ssl, Int32(bytesRead))
let result = CNIOBoringSSL_SSL_get_error(ssl, CInt(bytesRead))
let error = BoringSSLError.fromSSLGetErrorResult(result)!

switch error {
Expand All @@ -301,15 +301,15 @@ internal final class SSLConnection {
///
/// This call will only write the data into BoringSSL's internal buffers. It needs to be obtained
/// by calling `getDataForNetwork` after this call completes.
func writeDataToNetwork(_ data: inout ByteBuffer) -> AsyncOperationResult<Int32> {
func writeDataToNetwork(_ data: inout ByteBuffer) -> AsyncOperationResult<CInt> {
// BoringSSL does not allow calling SSL_write with zero-length buffers. Zero-length
// writes always succeed.
guard data.readableBytes > 0 else {
return .complete(0)
}

let writtenBytes = data.withUnsafeReadableBytes { (pointer) -> Int32 in
return CNIOBoringSSL_SSL_write(ssl, pointer.baseAddress, Int32(pointer.count))
let writtenBytes = data.withUnsafeReadableBytes { (pointer) -> CInt in
return CNIOBoringSSL_SSL_write(ssl, pointer.baseAddress, CInt(pointer.count))
}

if writtenBytes > 0 {
Expand Down Expand Up @@ -337,7 +337,7 @@ internal final class SSLConnection {
/// was negotiated.
func getAlpnProtocol() -> String? {
var protoName = UnsafePointer<UInt8>(bitPattern: 0)
var protoLen: UInt32 = 0
var protoLen: CUnsignedInt = 0

CNIOBoringSSL_SSL_get0_alpn_selected(ssl, &protoName, &protoLen)
guard protoLen > 0 else {
Expand Down
16 changes: 8 additions & 8 deletions Sources/NIOSSL/SSLContext.swift
Expand Up @@ -86,7 +86,7 @@ private func alpnCallback(ssl: OpaquePointer?,
outlen: UnsafeMutablePointer<UInt8>?,
in: UnsafePointer<UInt8>?,
inlen: UInt32,
appData: UnsafeMutableRawPointer?) -> Int32 {
appData: UnsafeMutableRawPointer?) -> CInt {
// Perform some sanity checks. We don't want NULL pointers around here.
guard let ssl = ssl, let out = out, let outlen = outlen, let `in` = `in` else {
return SSL_TLSEXT_ERR_NOACK
Expand Down Expand Up @@ -125,7 +125,7 @@ public final class NIOSSLContext {
guard boringSSLIsInitialized else { fatalError("Failed to initialize BoringSSL") }
guard let context = CNIOBoringSSL_SSL_CTX_new(CNIOBoringSSL_TLS_method()) else { throw NIOSSLError.unableToAllocateBoringSSLObject }

let minTLSVersion: Int32
let minTLSVersion: CInt
switch configuration.minimumTLSVersion {
case .tlsv13:
minTLSVersion = TLS1_3_VERSION
Expand All @@ -139,7 +139,7 @@ public final class NIOSSLContext {
var returnCode = CNIOBoringSSL_SSL_CTX_set_min_proto_version(context, UInt16(minTLSVersion))
precondition(1 == returnCode)

let maxTLSVersion: Int32
let maxTLSVersion: CInt

switch configuration.maximumTLSVersion {
case .some(.tlsv1):
Expand Down Expand Up @@ -270,7 +270,7 @@ extension NIOSSLContext {
private static func useCertificateChainFile(_ path: String, context: OpaquePointer) {
// TODO(cory): This shouldn't be an assert but should instead be actual error handling.
// assert(path.isFileURL)
let result = path.withCString { (pointer) -> Int32 in
let result = path.withCString { (pointer) -> CInt in
return CNIOBoringSSL_SSL_CTX_use_certificate_chain_file(context, pointer)
}

Expand Down Expand Up @@ -299,7 +299,7 @@ extension NIOSSLContext {

private static func usePrivateKeyFile(_ path: String, context: OpaquePointer) throws {
let pathExtension = path.split(separator: ".").last
let fileType: Int32
let fileType: CInt

switch pathExtension?.lowercased() {
case .some("pem"):
Expand All @@ -311,7 +311,7 @@ extension NIOSSLContext {
fatalError("Unknown private key file type.")
}

let result = path.withCString { (pointer) -> Int32 in
let result = path.withCString { (pointer) -> CInt in
return CNIOBoringSSL_SSL_CTX_use_PrivateKey_file(context, pointer, fileType)
}

Expand All @@ -325,7 +325,7 @@ extension NIOSSLContext {
// This copy should be done infrequently, so we don't worry too much about it.
let protoBuf = protocols.reduce([UInt8](), +)
let rc = protoBuf.withUnsafeBufferPointer {
CNIOBoringSSL_SSL_CTX_set_alpn_protos(context, $0.baseAddress!, UInt32($0.count))
CNIOBoringSSL_SSL_CTX_set_alpn_protos(context, $0.baseAddress!, CUnsignedInt($0.count))
}

// Annoyingly this function reverses the error convention: 0 is success, non-zero is failure.
Expand Down Expand Up @@ -377,7 +377,7 @@ extension NIOSSLContext {
throw NIOSSLError.noSuchFilesystemObject
}

let result = path.withCString { (pointer) -> Int32 in
let result = path.withCString { (pointer) -> CInt in
let file = !isDirectory ? pointer : nil
let directory = isDirectory ? pointer: nil
return CNIOBoringSSL_SSL_CTX_load_verify_locations(context, file, directory)
Expand Down
2 changes: 1 addition & 1 deletion Sources/NIOSSL/SSLErrors.swift
Expand Up @@ -131,7 +131,7 @@ public func ==(lhs: BoringSSLError, rhs: BoringSSLError) -> Bool {
}

internal extension BoringSSLError {
static func fromSSLGetErrorResult(_ result: Int32) -> BoringSSLError? {
static func fromSSLGetErrorResult(_ result: CInt) -> BoringSSLError? {
switch result {
case SSL_ERROR_NONE:
return .noError
Expand Down
2 changes: 1 addition & 1 deletion Sources/NIOSSL/SSLPrivateKey.swift
Expand Up @@ -166,7 +166,7 @@ public class NIOSSLPrivateKey {
/// A delegating initializer for `init(buffer:format:passphraseCallback)` and `init(buffer:format:)`.
private convenience init(bytes: [UInt8], format: NIOSSLSerializationFormats, callbackManager: CallbackManagerProtocol?) throws {
let ref = bytes.withUnsafeBytes { (ptr) -> UnsafeMutablePointer<EVP_PKEY>? in
let bio = CNIOBoringSSL_BIO_new_mem_buf(UnsafeMutableRawPointer(mutating: ptr.baseAddress!), Int32(ptr.count))!
let bio = CNIOBoringSSL_BIO_new_mem_buf(UnsafeMutableRawPointer(mutating: ptr.baseAddress!), CInt(ptr.count))!
defer {
CNIOBoringSSL_BIO_free(bio)
}
Expand Down
4 changes: 2 additions & 2 deletions Tests/NIOSSLTests/NIOSSLTestHelpers.swift
Expand Up @@ -292,7 +292,7 @@ func generateRSAPrivateKey() -> UnsafeMutablePointer<EVP_PKEY> {
return pkey
}

func addExtension(x509: UnsafeMutablePointer<X509>, nid: Int32, value: String) {
func addExtension(x509: UnsafeMutablePointer<X509>, nid: CInt, value: String) {
var extensionContext = X509V3_CTX()

CNIOBoringSSL_X509V3_set_ctx(&extensionContext, x509, x509, nil, nil, 0)
Expand Down Expand Up @@ -335,7 +335,7 @@ func generateSelfSignedCert() -> (NIOSSLCertificate, NIOSSLPrivateKey) {
NID_commonName,
MBSTRING_UTF8,
UnsafeMutablePointer(mutating: pointer),
Int32(commonName.lengthOfBytes(using: .utf8)),
CInt(commonName.lengthOfBytes(using: .utf8)),
-1,
0)
}
Expand Down

0 comments on commit c8ac6c1

Please sign in to comment.