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

espconn lwip wrappers actively harmful #3004

Open
nwf opened this issue Jan 5, 2020 · 4 comments
Open

espconn lwip wrappers actively harmful #3004

nwf opened this issue Jan 5, 2020 · 4 comments

Comments

@nwf
Copy link
Member

nwf commented Jan 5, 2020

As far as I can tell, anyway, these things are just obstruction, rather than abstraction, over lwip. See, for example, the damage they cause with #2987: it is neither obvious nor documented that espconn_gethostbyname passes the espconn structure as the callback argument. We should just be calling dns_gethostbyname ourselves and passing the appropriate lua object to the callback.

Am I missing something and espconn is actually useful? If not, I think I will propose that we attempt to excise it from our tree.

nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jan 6, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Jan 6, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
@nwf nwf mentioned this issue Jan 6, 2020
4 tasks
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 2, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 2, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 2, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 2, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 3, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 3, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 23, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 23, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 23, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 23, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 25, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 25, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 25, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Feb 25, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Mar 31, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Mar 31, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Apr 2, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Apr 2, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Apr 5, 2020
This is the easiest part of nodemcu#3004 .
It removes a bunch of functions that were never called in our tree.
nwf added a commit to nwf/nodemcu-firmware that referenced this issue Apr 5, 2020
Further work on nodemcu#3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.
nwf added a commit that referenced this issue Apr 7, 2020
* espconn: remove unused espconn code, take 1

This is the easiest part of #3004 .
It removes a bunch of functions that were never called in our tree.

* espconn: De-orbit espconn_gethostbyname

Further work on #3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.

* espconn: remove scary global pktinfo

A write-only global!  How about that.

* net: remove deprecated methods

All the TLS stuff moved over there a long time ago, and
net_createUDPSocket should just do what it says on the tin.

* espconn_secure: remove ESPCONN_SERVER support

We can barely function as a TLS client; being a TLS server seems like a
real stretch.  This code was never called from Lua anyway.

* espconn_secure: more code removal

* espconn_secure: simplify ssl options structure

There is nothing "ssl_packet" about this structure.  Get rid of the
terrifying "pbuffer" pointer.

Squash two structure types together and eliminate an unused field.

* espconn_secure: refactor mbedtls_msg_info_load

Split out espconn_mbedtls_parse, which we can use as part of our effort
towards addressing #3032

* espconn_secure: introduce TLS cert/key callbacks

The new feature part of #3032
Subsequent work will remove the old mechanism.

* tls: add deprecation warnings

* luacheck: net.ifinfo is a thing now

* tls: remove use of espconn->reverse

* mqtt: stop using espconn->reverse

Instead, just place the espconn structure itself at the top of the user
data.  This enlarges the structure somewhat but removes one more layer
of dynamic heap usage and NULL checks.

While here, simplify the code a bit.

* mqtt: remove redundant pointer to connect_info

Everywhere we have the mqtt_state_t we also have the lmqtt_userdata.

* mqtt: doc fixes

* mqtt: note bug

* tls: allow :on(...,nil) to unregister a callback
marcelstoer pushed a commit that referenced this issue Jun 9, 2020
* espconn: remove unused espconn code, take 1

This is the easiest part of #3004 .
It removes a bunch of functions that were never called in our tree.

* espconn: De-orbit espconn_gethostbyname

Further work on #3004

While here, remove `mqtt`'s charming DNS-retry logic (which is neither
shared with nor duplicated in other modules) and update its :connect()
return value behavior and documentation.

* espconn: remove scary global pktinfo

A write-only global!  How about that.

* net: remove deprecated methods

All the TLS stuff moved over there a long time ago, and
net_createUDPSocket should just do what it says on the tin.

* espconn_secure: remove ESPCONN_SERVER support

We can barely function as a TLS client; being a TLS server seems like a
real stretch.  This code was never called from Lua anyway.

* espconn_secure: more code removal

* espconn_secure: simplify ssl options structure

There is nothing "ssl_packet" about this structure.  Get rid of the
terrifying "pbuffer" pointer.

Squash two structure types together and eliminate an unused field.

* espconn_secure: refactor mbedtls_msg_info_load

Split out espconn_mbedtls_parse, which we can use as part of our effort
towards addressing #3032

* espconn_secure: introduce TLS cert/key callbacks

The new feature part of #3032
Subsequent work will remove the old mechanism.

* tls: add deprecation warnings

* luacheck: net.ifinfo is a thing now

* tls: remove use of espconn->reverse

* mqtt: stop using espconn->reverse

Instead, just place the espconn structure itself at the top of the user
data.  This enlarges the structure somewhat but removes one more layer
of dynamic heap usage and NULL checks.

While here, simplify the code a bit.

* mqtt: remove redundant pointer to connect_info

Everywhere we have the mqtt_state_t we also have the lmqtt_userdata.

* mqtt: doc fixes

* mqtt: note bug

* tls: allow :on(...,nil) to unregister a callback
@stale
Copy link

stale bot commented Jan 31, 2021

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 31, 2021
@nwf
Copy link
Member Author

nwf commented Jan 31, 2021

Still true, sadly.

@stale stale bot removed the stale label Jan 31, 2021
@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Apr 16, 2022
@HHHartmann
Copy link
Member

Is it still true?

@stale stale bot removed the stale label Apr 19, 2022
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

No branches or pull requests

2 participants