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

Quell some warnings #10023

Merged
merged 2 commits into from Jun 23, 2016
Merged

Quell some warnings #10023

merged 2 commits into from Jun 23, 2016

Conversation

notspiff
Copy link
Contributor

  • redefinition of HAVE_OPENSSL with cmake builds (comes from command line not config.h)
  • avoid deprecated symbol usage in mhd with newer versions

@Montellese for the last one i guess.

@MartijnKaijser
Copy link
Member

or @Paxxi ping

Our depends/win32 all have MHD 0.9.50 so this is just for the distros that have a lower version correct?

@notspiff
Copy link
Contributor Author

Well the old code is kept for that reason. it quells warnings which
appeared in 0.9.4
23. jun. 2016 08:50 skrev "Martijn Kaijser" notifications@github.com:

or @Paxxi https://github.com/Paxxi ping

Our depends/win32 all have MHD 0.9.50 so this is just for the distros that
have a lower version correct?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#10023 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AFktYPZm26MDJaeKZLR-GwMcZ6KpURb0ks5qOizQgaJpZM4I8fi7
.

mode = MHD_RESPMEM_MUST_COPY;
else if (free)
mode = MHD_RESPMEM_MUST_FREE;
struct MHD_Response *response = MHD_create_response_from_buffer(0, nullptr, mode);

This comment was marked as spam.

@Montellese
Copy link
Member

I'm not really up-to-date with the development of libmicrohttpd but the changes look ok.

@@ -102,9 +102,25 @@ CWebServer::CWebServer()
#endif
}

static MHD_Response* create_response(size_t size, void* data, int free, int copy)

This comment was marked as spam.

@Paxxi
Copy link
Member

Paxxi commented Jun 23, 2016

good for merge imo unless @notspiff agrees about the enum and want to change it :)

@akva2
Copy link
Contributor

akva2 commented Jun 23, 2016

If you think investing work in the legacy code path is worth it.. it has been ints for years due to mhd api. Basically your comment is what was adressed upstream.

@notspiff
Copy link
Contributor Author

early returns added. i will do the enumeration only if you insist. when we drop the legacy code path we can directly use the mhd enum so it feels kinda unnnecessary to put work into it.

@Paxxi
Copy link
Member

Paxxi commented Jun 23, 2016

@notspiff not needed, as I mentioned in previous comment I left the decision to you as it's your time and a minor thing.

@Paxxi Paxxi merged commit 6bf535d into xbmc:master Jun 23, 2016
@Paxxi Paxxi added the Type: Cleanup non-breaking change which removes non-working or unmaintained functionality label Jun 23, 2016
@Paxxi Paxxi added this to the Krypton 17.0-alpha2 milestone Jun 23, 2016
@L-S-D
Copy link

L-S-D commented Jun 24, 2016

FYI, this breaks at least the IOS Remote and in general the WebIF for me

@notspiff
Copy link
Contributor Author

if you are going to FYI me, you better give me some information ;)

@notspiff notspiff deleted the quell_warnings_2 branch June 24, 2016 07:47
@L-S-D
Copy link

L-S-D commented Jun 24, 2016

yeah you are absolutely right, just made my wife happy in finding the PR that broke it. For now I just can say that IOS Remote cannot connect to kodi anymore. I'm running on non standard ports. I won't be able to make logs and dig into that deeper until in the evening.

@L-S-D
Copy link

L-S-D commented Jun 24, 2016

working fine with libmicrohttpd 0.9.37 on debian (default for jessie) and fails with 0.9.44 from stretch

mode = MHD_RESPMEM_MUST_FREE;
return MHD_create_response_from_buffer(0, nullptr, mode);
#else
return MHD_create_response_from_data(0, nullptr, free, copy);

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

@MilhouseVH
Copy link
Contributor

Hi, just some additional detail on what is not working.

iOS and Yatse remote control apps no longer work after this change (Ubuntu x86_64 16.04, LibreELEC #623 RPi and x86).

Connecting to the webgui on port 8080 with a browser will result in "Connection to server lost" messages.

The following JSON query will never return a result, it simply hangs - it should return the JSON schema more or less instantly:

curl -sG -H 'content-type: application/json;' http://192.168.0.18:8080/jsonrpc

iOS and Yatse applications can't create connections to clients because the webserver does not respond with the above query.

Reverting this change restores functionality.

I'll try a build with the change suggested by @popcornmix.

popcornmix added a commit to popcornmix/xbmc that referenced this pull request Jun 24, 2016
popcornmix added a commit to popcornmix/xbmc that referenced this pull request Jun 24, 2016
popcornmix added a commit that referenced this pull request Jun 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Cleanup non-breaking change which removes non-working or unmaintained functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants