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

Fix warnings #45

Merged
merged 3 commits into from
Jun 11, 2020
Merged

Fix warnings #45

merged 3 commits into from
Jun 11, 2020

Conversation

cyrusingraham
Copy link
Contributor

@cyrusingraham cyrusingraham commented Jun 5, 2020

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
Copy link

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
Copy link

levidhuyvetter commented Jun 6, 2020

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
Copy link
Contributor Author

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

@jakeheis jakeheis mentioned this pull request Jun 11, 2020
@jakeheis jakeheis merged commit 83b1d6a into jakeheis:master Jun 11, 2020
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

Successfully merging this pull request may close these issues.

3 participants