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

Various fixes on #866 which broke building for iOS and Mac #888

Merged
merged 7 commits into from
Oct 9, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions Release/include/cpprest/http_compression.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class decompress_factory
{
public:
virtual const utility::string_t& algorithm() const = 0;
virtual const uint16_t weight() const = 0;
virtual uint16_t weight() const = 0;
virtual std::unique_ptr<decompress_provider> make_decompressor() const = 0;
virtual ~decompress_factory() = default;
};
Expand All @@ -117,9 +117,9 @@ _ASYNCRTIMP bool supported();
/// </summary>
namespace algorithm
{
constexpr utility::char_t *GZIP = _XPLATSTR("gzip");
constexpr utility::char_t *DEFLATE = _XPLATSTR("deflate");
constexpr utility::char_t *BROTLI = _XPLATSTR("br");
constexpr utility::char_t *GZIP = const_cast<utility::char_t *>(_XPLATSTR("gzip"));
constexpr utility::char_t *DEFLATE = const_cast<utility::char_t *>(_XPLATSTR("deflate"));
constexpr utility::char_t *BROTLI = const_cast<utility::char_t *>(_XPLATSTR("br"));

/// <summary>
/// Test whether cpprestsdk was built with built-in compression support and
Expand All @@ -129,7 +129,7 @@ constexpr utility::char_t *BROTLI = _XPLATSTR("br");
/// the supplied string matches a supported built-in algorithm, and false if not.</returns>
/// <summary>
_ASYNCRTIMP bool supported(const utility::string_t& algorithm);
};
}

/// <summary>
/// Factory function to instantiate a built-in compression provider with default parameters by compression algorithm
Expand Down
62 changes: 31 additions & 31 deletions Release/include/cpprest/http_headers.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,36 @@
#include "cpprest/asyncrt_utils.h"

namespace web { namespace http {
namespace details
{
template<typename key_type, typename _t>
bool bind_impl(const key_type &text, _t &ref)
{
utility::istringstream_t iss(text);
iss.imbue(std::locale::classic());
iss >> ref;
if (iss.fail() || !iss.eof())
{
return false;
}

return true;
}

template<typename key_type>
bool bind_impl(const key_type &text, utf16string &ref)
{
ref = utility::conversions::to_utf16string(text);
return true;
}

template<typename key_type>
bool bind_impl(const key_type &text, std::string &ref)
{
ref = utility::conversions::to_utf8string(text);
return true;
}
}

/// <summary>
/// Binds an individual reference to a string value.
Expand Down Expand Up @@ -219,7 +249,7 @@ class http_headers
return false;
}

return details::bind_impl(iter->second, value) || iter->second.empty();
return web::http::details::bind_impl(iter->second, value) || iter->second.empty();
}

/// <summary>
Expand Down Expand Up @@ -289,34 +319,4 @@ class http_headers
inner_container m_headers;
};

namespace details
{
template<typename key_type, typename _t>
bool bind_impl(const key_type &text, _t &ref)
{
utility::istringstream_t iss(text);
iss.imbue(std::locale::classic());
iss >> ref;
if (iss.fail() || !iss.eof())
{
return false;
}

return true;
}

template<typename key_type>
bool bind_impl(const key_type &text, utf16string &ref)
{
ref = utility::conversions::to_utf16string(text);
return true;
}

template<typename key_type>
bool bind_impl(const key_type &text, std::string &ref)
{
ref = utility::conversions::to_utf8string(text);
return true;
}
}
}}
4 changes: 4 additions & 0 deletions Release/src/http/client/http_client_asio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1753,7 +1753,11 @@ class asio_context final : public request_context, public std::enable_shared_fro
writeBuffer.putn_nocopy(shared_decompressed->data(), shared_decompressed->size())
.then([this_request, read_size, shared_decompressed AND_CAPTURE_MEMBER_FUNCTION_POINTERS](
pplx::task<size_t> op) {
#if defined(__GNUC__)
size_t writtenSize __attribute__ ((unused)) = 0;
#else // __GNUC__
size_t writtenSize = 0;
#endif // __GNUC__
try
{
writtenSize = op.get();
Expand Down
70 changes: 46 additions & 24 deletions Release/src/http/common/http_compression.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class zlib_compressor_base : public compress_provider

private:
int m_state{Z_BUF_ERROR};
z_stream m_stream{0};
Copy link
Contributor

Choose a reason for hiding this comment

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

What issue did this initializer cause? And will this empty initializer ensure that all members of the z_stream struct are initialized to zero? I think it might, but I'm not confident enough in my reading of the spec on that point to swear to it.

If not, then we'll need a memset to zero in the zlib_compressor_base (and zlib_decompressor_base) constructor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the errors are listed in #887. This one in particular generated

Release/src/http/common/http_compression.cpp:159:24: error: missing field 'avail_in' initializer
      [-Werror,-Wmissing-field-initializers]
    z_stream m_stream{0};

I was surprised by this one as well.

https://stackoverflow.com/questions/1538943/why-is-the-compiler-throwing-this-warning-missing-initializer-isnt-the-stru

Seems like it is, as the one poster in that SO put it, "GCC is just being overly paranoid". That posting is old, but yet it seems like the issue still exists. It looks like there have been requests to change it as well.

Let me know how you want this handled, in the meantime I'll make the other changes you listed above later today.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I fixed the two items you mentioned. I'm not going to mark this as resolved ... if you feel this is suitable, then you can mark it as such. Or let me know what you'd like to see.

Copy link
Contributor

@epistor epistor Oct 9, 2018

Choose a reason for hiding this comment

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

The links are helpful. I don't seem to have a button available to mark this as resolved; possibly just ignorance on my part?

I'd like @BillyONeal to confirm, though, anyway -- and he'll have to review this, regardless, as I don't have permission to merge it.

z_stream m_stream{};
const utility::string_t& m_algorithm;
};

Expand Down Expand Up @@ -263,7 +263,7 @@ class zlib_decompressor_base : public decompress_provider

private:
int m_state{Z_BUF_ERROR};
z_stream m_stream{0};
z_stream m_stream{};
const utility::string_t& m_algorithm;
};

Expand All @@ -283,7 +283,7 @@ class gzip_compressor : public zlib_compressor_base
class gzip_decompressor : public zlib_decompressor_base
{
public:
gzip_decompressor::gzip_decompressor() : zlib_decompressor_base(16) // gzip auto-detect
gzip_decompressor() : zlib_decompressor_base(16) // gzip auto-detect
{
}
};
Expand Down Expand Up @@ -620,7 +620,7 @@ class generic_decompress_factory : public decompress_factory

const utility::string_t& algorithm() const { return m_algorithm; }

const uint16_t weight() const { return m_weight; }
uint16_t weight() const { return m_weight; }

std::unique_ptr<decompress_provider> make_decompressor() const { return _make_decompressor(); }

Expand All @@ -634,14 +634,14 @@ class generic_decompress_factory : public decompress_factory
static const std::vector<std::shared_ptr<compress_factory>> g_compress_factories
#if defined(CPPREST_HTTP_COMPRESSION)
= {std::make_shared<generic_compress_factory>(
algorithm::GZIP, []() -> std::unique_ptr<compress_provider> { return std::make_unique<gzip_compressor>(); }),
algorithm::GZIP, []() -> std::unique_ptr<compress_provider> { return utility::details::make_unique<gzip_compressor>(); }),
std::make_shared<generic_compress_factory>(
algorithm::DEFLATE,
[]() -> std::unique_ptr<compress_provider> { return std::make_unique<deflate_compressor>(); }),
[]() -> std::unique_ptr<compress_provider> { return utility::details::make_unique<deflate_compressor>(); }),
#if defined(CPPREST_BROTLI_COMPRESSION)
std::make_shared<generic_compress_factory>(
algorithm::BROTLI,
[]() -> std::unique_ptr<compress_provider> { return std::make_unique<brotli_compressor>(); })
[]() -> std::unique_ptr<compress_provider> { return utility::details::make_unique<brotli_compressor>(); })
#endif // CPPREST_BROTLI_COMPRESSION
};
#else // CPPREST_HTTP_COMPRESSION
Expand All @@ -653,16 +653,16 @@ static const std::vector<std::shared_ptr<decompress_factory>> g_decompress_facto
= {std::make_shared<generic_decompress_factory>(
algorithm::GZIP,
500,
[]() -> std::unique_ptr<decompress_provider> { return std::make_unique<gzip_decompressor>(); }),
[]() -> std::unique_ptr<decompress_provider> { return utility::details::make_unique<gzip_decompressor>(); }),
std::make_shared<generic_decompress_factory>(
algorithm::DEFLATE,
500,
[]() -> std::unique_ptr<decompress_provider> { return std::make_unique<deflate_decompressor>(); }),
[]() -> std::unique_ptr<decompress_provider> { return utility::details::make_unique<deflate_decompressor>(); }),
#if defined(CPPREST_BROTLI_COMPRESSION)
std::make_shared<generic_decompress_factory>(
algorithm::BROTLI,
500,
[]() -> std::unique_ptr<decompress_provider> { return std::make_unique<brotli_decompressor>(); })
[]() -> std::unique_ptr<decompress_provider> { return utility::details::make_unique<brotli_decompressor>(); })
#endif // CPPREST_BROTLI_COMPRESSION
};
#else // CPPREST_HTTP_COMPRESSION
Expand Down Expand Up @@ -748,32 +748,54 @@ std::shared_ptr<decompress_factory> get_decompress_factory(const utility::string
return std::shared_ptr<decompress_factory>();
}


#if defined(CPPREST_HTTP_COMPRESSION)
std::unique_ptr<compress_provider> make_gzip_compressor(int compressionLevel, int method, int strategy, int memLevel)
{
#if defined(CPPREST_HTTP_COMPRESSION)
return std::move(std::make_unique<gzip_compressor>(compressionLevel, method, strategy, memLevel));
return utility::details::make_unique<gzip_compressor>(compressionLevel, method, strategy, memLevel);
}
#else // CPPREST_HTTP_COMPRESSION
#if defined(__GNUC__)
std::unique_ptr<compress_provider> make_gzip_compressor(int compressionLevel __attribute__((unused)), int method __attribute__((unused)), int strategy __attribute__((unused)), int memLevel __attribute__((unused)))
#else // __GNUC__
std::unique_ptr<compress_provider> make_gzip_compressor(int compressionLevel, int method, int strategy, int memLevel)
#endif // __GNUC__
{
return std::unique_ptr<compress_provider>();
#endif // CPPREST_HTTP_COMPRESSION
}
#endif // CPPREST_HTTP_COMPRESSION

#if defined(CPPREST_HTTP_COMPRESSION)
std::unique_ptr<compress_provider> make_deflate_compressor(int compressionLevel, int method, int strategy, int memLevel)
{
#if defined(CPPREST_HTTP_COMPRESSION)
return std::move(std::make_unique<deflate_compressor>(compressionLevel, method, strategy, memLevel));
return utility::details::make_unique<deflate_compressor>(compressionLevel, method, strategy, memLevel);
}
#else // CPPREST_HTTP_COMPRESSION
#if defined(__GNUC__)
std::unique_ptr<compress_provider> make_deflate_compressor(int compressionLevel __attribute__((unused)), int method __attribute__((unused)), int strategy __attribute__((unused)), int memLevel __attribute__((unused)))
#else // __GNUC__
std::unique_ptr<compress_provider> make_deflate_compressor(int compressionLevel, int method, int strategy, int memLevel)
#endif // __GNUC__
{
return std::unique_ptr<compress_provider>();
#endif // CPPREST_HTTP_COMPRESSION
}
#endif // CPPREST_HTTP_COMPRESSION

#if defined(CPPREST_HTTP_COMPRESSION) && defined(CPPREST_BROTLI_COMPRESSION)
std::unique_ptr<compress_provider> make_brotli_compressor(uint32_t window, uint32_t quality, uint32_t mode)
{
#if defined(CPPREST_HTTP_COMPRESSION) && defined(CPPREST_BROTLI_COMPRESSION)
return std::move(std::make_unique<brotli_compressor>(window, quality, mode));
return utility::details::make_unique<brotli_compressor>(window, quality, mode);
}
#else // CPPREST_BROTLI_COMPRESSION
#if defined(__GNUC__)
std::unique_ptr<compress_provider> make_brotli_compressor(uint32_t window __attribute__((unused)), uint32_t quality __attribute__((unused)), uint32_t mode __attribute__((unused)))
#else // __GNUC__
std::unique_ptr<compress_provider> make_brotli_compressor(uint32_t window, uint32_t quality, uint32_t mode)
#endif // __GNUC__
{
return std::unique_ptr<compress_provider>();
#endif // CPPREST_BROTLI_COMPRESSION
}
#endif // CPPREST_BROTLI_COMPRESSION
} // namespace builtin

std::shared_ptr<compress_factory> make_compress_factory(
Expand Down Expand Up @@ -951,7 +973,7 @@ std::unique_ptr<compress_provider> get_compressor_from_header(

if (compressor)
{
return std::move(compressor);
return compressor;
}

// If we're here, we didn't match the caller's compressor above;
Expand All @@ -965,7 +987,7 @@ std::unique_ptr<compress_provider> get_compressor_from_header(
auto compressor = web::http::compression::builtin::_make_compressor(f, coding);
if (compressor)
{
return std::move(compressor);
return compressor;
}
if (type == header_types::accept_encoding && utility::details::str_iequal(coding, _XPLATSTR("identity")))
{
Expand Down Expand Up @@ -1068,7 +1090,7 @@ std::unique_ptr<decompress_provider> get_decompressor_from_header(

// Either the response is compressed and we have a decompressor that can handle it, or
// built-in compression is not enabled and we don't have an alternate set of decompressors
return std::move(decompressor);
return decompressor;
}

utility::string_t build_supported_header(header_types type,
Expand All @@ -1084,7 +1106,7 @@ utility::string_t build_supported_header(header_types type,
// Add all specified algorithms and their weights to the header
start = true;
os.imbue(std::locale::classic());
for each (auto& factory in f)
for (auto& factory : f)
{
if (factory)
{
Expand All @@ -1109,7 +1131,7 @@ utility::string_t build_supported_header(header_types type,
os << _XPLATSTR("identity;q=1, *;q=0");
}

return std::move(os.str());
return os.str();
}
} // namespace details
} // namespace compression
Expand Down
2 changes: 1 addition & 1 deletion Release/src/http/common/internal_http_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ bool validate_method(const utility::string_t& method);

namespace web { namespace http { namespace compression {

class compression::decompress_factory;
class decompress_factory;

namespace details { namespace builtin {

Expand Down
Loading