jakeheis / Shout

SSH made easy in Swift
MIT License
357 stars 103 forks source link

Fix warnings #45

Closed cyrusingraham closed 4 years ago

cyrusingraham commented 4 years ago

Most of the warnings were due to withUnsafeBytes with UnsafePointer as a block parameter being deprecated. In the new code, instead of force unwrapping to get the baseAddress I used default return values. I can change the values or force unwrap the baseAddress if you would like.

levidhuyvetter commented 4 years ago

Perhaps the default return values should be a negative number to indicate an error? This would then be caught (and later thrown) as an error in:

static func processRead(result: Int, buffer: inout [Int8], session: OpaquePointer) -> ReadResult {
        if result > 0 {
            let data = Data(buffer: UnsafeBufferPointer(start: &buffer, count: result))
            return .data(data)
        } else if result == 0 {
            return .done
        } else if result == LIBSSH2_ERROR_EAGAIN {
            return .eagain
        } else {
            return .error(SSHError.codeError(code: Int32(result), session: session))
        }
    }
levidhuyvetter commented 4 years ago

Hi Cyrus,

Not sure where your comment went about the error codes being positive numbers (only got it by email) but they're being flipped by the SSHError struct:

static func codeError(code: Int32, session: OpaquePointer) -> SSHError {
    return SSHError(kind: Kind(rawValue: -code) ?? .genericError, session: session)
}

So result should be a negative number on error as 0 or any positive just indicates the amount of bytes read/written.

Looking through the options there, not to conflict with anything, I would go with a -1 which will result in a SSHError.genericError being thrown.

Perhaps to make debugging easier, in processRead and processWrite, you can do a check and return an SSHError.genericError with a custom message about memory gone missing?

Also where you're returning an empty string in execute(command,output) when binding fails, you could throw this same error.

cyrusingraham commented 4 years ago

Hi Levi, yes I deleted the comment after after posting because I realized I was incorrect for the reason you mentioned.