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

More, but more subdued, rampaging for next release #3064

Merged
merged 11 commits into from
Apr 19, 2020
Merged

Conversation

nwf
Copy link
Member

@nwf nwf commented Apr 10, 2020

  • This PR is for the dev branch rather than for master.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

This is another round of minor scattered changes throughout the tree. I'd like at least tls: fix new verification API to land because it corrects an embarrassing error on my part.

I'd appreciate someone glancing over this, especially some of the more obscure Lua bits (__metatable, LUA_GCSTOP, ...).

Copy link
Member

@HHHartmann HHHartmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also had a look at the mispec and lua changes.
But I don't understand the rest of your changes (yet)

app/modules/net.c Show resolved Hide resolved
app/modules/net.c Show resolved Hide resolved
nwf added 11 commits April 19, 2020 15:19
The additional reporting, while nice, prevents the use of mispec on
integer-only builds
Prefer print everywhere.
IMHO, it's generally good style to register the callbacks first.
Seems more polite than quietly accepting other types as nil.
Removes yet another unchecked allocation point in our C libraries.

While here, fix potential reference leaks on error paths

Also while here, remove some stale documentation.  There can be as many
DNS requests in flight as LwIP has room for in its table
(DNS_TABLE_SIZE, which defaults to 4).
Because the old API was inactive, we were setting
MBEDTLS_SSL_VERIFY_NONE even after we'd parsed the certificate.

tls tests now include a deliberate certificate mismatch; this was
discovered by moving the mqtt tests over to the new API.
@nwf
Copy link
Member Author

nwf commented Apr 19, 2020

Sorry about that; pushed the wrong set of commits. D'oh. Anyway, here's the correct set with @HHHartmann's suggestion in place. I'd like to merge these this weekend, so if I don't hear any objections by this evening I'll click my own button and will deal with any fallout. :)

@nwf nwf merged commit 04ab7d7 into nodemcu:dev Apr 19, 2020
@nwf nwf deleted the for-upstream branch April 19, 2020 18:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants