Just wanted to gently nudge this thread.

As mentioned, I'm happy to do any required cleanup on this patch, but I'd appreciate a review by anyone who knows the ssh.c code better than me.

Fwiw it seems to be working ok for us, although we probably don't have coverage in our tests of everything that I've modified.

I've tried running the full libcurl test suite, however, and I'm getting these failures:

        530 564 584 591 592 604 615 622 706 1300 1301 1302 1303 1304 1305 1306 1307 1308 1309 1316

With a plain vanilla build that doesn't include my changes, the following tests still fail:

        564 706 1300 1301 1302 1303 1304 1305 1306 1307 1308 1309

So I guess there's perhaps something wrong with my test setup (?), but potentially these test cases may be affected by my change:

        530 584 591 592 604 615 622 1316

I'm not entirely sure where to find out what these various tests are and what might be failing (I thought each one might be a file in tests/libtest/*.c, but not all of the test numbers seem to correspond to one of these).

Any pointers welcome on that front.

Cheers.

- Sam -

sam deane / @samdeane | elegantchaos.com / @elegantchaoscom | mac and ios software development

On 11 Apr 2013, at 17:03, Sam Deane <sam_at_elegantchaos.com> wrote:

> On 10 Apr 2013, at 23:30, Dan Fandrich <dan_at_coneharvesters.com> wrote:
>
>> That's what's returned by CURLINFO_RESPONSE_CODE, which is already used for
>> non-HTTP protocols, so it sounds like a reasonable extension to me. Please also
>> document this use in curl_easy_getinfo.3 Ideally, the response code for all
>> SFTP operations would be stored, not just failures in quoted commands. And
>> httpcode must be 0 in all cases when the command succeeds.
>
> Here's another patch which is a more comprehensive stab at the task, although it should still be regarded as work in progress.
>
> I've attempted to replace manual setting of conn->proto.sshc.actualcode with a function that also sets the httpCode.
>
> Unfortunately there was an awful lot of repeated code in ssh.c relating to error reporting and/or failure. I've attempted to replace the most obvious blocks of repeated code with functions, but there's still a lot of repetition or nearly-but-not-quite-the-same code.
>
> Some of this refactoring worried me a little bit - since the likelihood is high that somewhere in this process a small mistake will have crept in - but it was the only sane way that I could see to approach the job.
>
> I suspect that there are still quite a few places where the code could be generalised further, allowing me to remove some of the variants of my error/failure functions.
>
> For example, there are lots of places in the code where the last ssh error wasn't checked. I've used LIBSSH2_ERROR_NONE for the ssh error in these cases, because I wasn't sure it if would always be safe to call sftp_libssh2_last_error in places where it wasn't being called before.
>
> Someone who is more familiar with this code should probably review my changes. You will probably also be able to suggest some further improvements or simplifications - feel free to do so and I'll try to apply them.
>
> <patch.diff>
>
> Haven't updated the docs yet, but I will do so once we're fairly sure it's working.
>
> - Sam -
>
>
> sam deane / @samdeane | elegantchaos.com / @elegantchaoscom | mac and ios software development
>
> -------------------------------------------------------------------
> List admin: http://cool.haxx.se/list/listinfo/curl-library
> Etiquette: http://curl.haxx.se/mail/etiquette.html

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2013-05-13