lwip - Fixed error codes for failed TCP connect

[geky]

Some error codes in TCP connect were being remapped incorrectly

condition               posix error     mbed error
good host, closed port  ECONNREFUSED    NSAPI_ERROR_NO_CONNECTION
bad host                EHOSTUNREACH    NSAPI_ERROR_NO_CONNECTION
bad network             ENETUNREACH     NSAPI_ERROR_NO_CONNECTION

Thanks to @infinnovation for finding this, verified with their test program in #3246

cc @infinnovation, @c1728p9
should resolve #3246

[bridadan]

Nice!

[bridadan]

/morph test-nightly

[mbed-bot]

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1232

All builds and test passed!

[kjbracey]

Seems correct, but as I've just been working in the area with Nanostack's TCP and been studying this...

There are two more common fatal error conditions to consider:

condition                            posix error     mbed error
remote host not responding           ETIMEDOUT       NSAPI_ERROR_NO_CONNECTION
we or remote host aborted            ECONNRESET      NSAPI_ERROR_NO_CONNECTION

Presumably ECONNRESET and ECONNREFUSED are both ERR_RST, but not sure if timeout falls in there too.

[adbridge]

@geky Hi Chris there are a couple of things here. Firstly could this be a breaking change as one of the return codes has a different value now. Secondly do you want to address Kevin's comments? Currently postponing this to 5.4 in the meantime.

[geky]

The change is to return an error instead of succeeding when a connection fails. Later usage of the socket would result in (hopefully) an error anyways. I'm not sure how this could break backwards compatibility short of applications intentionally creating invalid connections.

@kjbracey-arm, I haven't had the chance to test the conditions you point out. Do you think we should add the additionally error codes for better granularity as to what went wrong in a connection? My concern is that it's unlikely the small devices (esp8266, mx, c027...) will return anything more than "connection failed". Do you think having the option for granularity is still worthwhile?

[kjbracey]

See also my comments on #3457 with regard to error codes. I don't see any particular need to distinguish between all the errors listed here, but I am a bit wary about the lack of a distinction between "not connected [yet] (ENOTCONN)" and all the failure codes once we have non-blocking connect.

Separate issue from this patch, and any splitting of error codes is a potential backwards compatibility break, so would need more thought. I think this patch is fine.

All the problem-type codes are "event"/"sticky" ones generally returned only once by the stack via SO_ERROR, and then calls will revert back to the base ENOTCONN. It's certainly possible that some implementations couldn't provide them at all and just give ENOTCONN. But maybe stacks that can could return new ones before falling back to NO_CONNECTION.

[sg-]

@adbridge did you forget to merge or is something else needed here?

[adbridge]

Looks like a 'missed the merge step' !