Skip to content

Conversation

@brfransen
Copy link

Fixes to errors encountered while building with newer deps.

Copy link
Member

@Risca Risca left a comment

Choose a reason for hiding this comment

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

Apart from the missing headerValues, it looks good to me. Haven't tested building or running it. Just did a quick scan


ret = MHD_add_response_header(response, MHD_HTTP_HEADER_WWW_AUTHENTICATE, "Basic realm=XBMC");
ret |= MHD_add_response_header(response, MHD_HTTP_HEADER_CONNECTION, "close");
ret = MHD_add_response_header(response, MHD_HTTP_HEADER_CONNECTION, "close");
Copy link
Member

Choose a reason for hiding this comment

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

We don't need "Basic realm=XBMC" header?

return MHD_NO;

return MHD_get_connection_values(connection, kind, FillArgumentMap, &headerValues);
return MHD_YES;
Copy link
Member

Choose a reason for hiding this comment

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

I don't see headerValues being populated any more. This looks like a bug

return MHD_NO;

return MHD_get_connection_values(connection, kind, FillArgumentMultiMap, &headerValues);
return MHD_YES;
Copy link
Member

Choose a reason for hiding this comment

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

Same here: I don't see headerValues being populated

HandleEvents();
if(m_recorder)
return m_dll->recorder_is_recording(m_recorder) > 0;
return m_dll->recorder_is_recording(m_recorder) > (void *)0;
Copy link
Member

Choose a reason for hiding this comment

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

I've always preferred the implicit conversion of pointers to boolean. E.g.;

return !!m_dll->recorder_is_recording(m_recorder);

or

if (CGUIDialogNumeric::ShowAndGetTime(time, heading ? heading : g_localizeStrings.Get(21420)))

but this works too ^^

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

Successfully merging this pull request may close these issues.

2 participants