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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
09714d9
Various fixes on #866 which broke building for iOS and Mac
Oct 7, 2018
b0927b9
Remove trailing semicolon from namespace since flags error when build…
Oct 7, 2018
d2b8032
Ubuntu builds needs to factor in unused arguments/variables
Oct 8, 2018
893c101
Remove trailing spaces from empty lines
Oct 8, 2018
8d80a36
Merge branch 'master' into issue_887
BillyONeal Oct 9, 2018
ba20325
Remove duplicate bind_impls.
BillyONeal Oct 9, 2018
790e620
Fix some merge fallout with master.
BillyONeal Oct 9, 2018
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
3 changes: 1 addition & 2 deletions Release/include/cpprest/http_headers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "cpprest/asyncrt_utils.h"

namespace web { namespace http {

/// <summary>
/// Binds an individual reference to a string value.
/// </summary>
Expand Down Expand Up @@ -250,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
35 changes: 18 additions & 17 deletions Release/src/http/common/http_compression.cpp
View file
Open in desktop
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.

Oh and sorry for not directly replying. They should be the same.

https://stackoverflow.com/questions/41006259/brace-initialization-difference-betweein-and-0-in-c

https://en.cppreference.com/w/cpp/language/value_initialization

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 •
edited
Loading

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 @@ -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,10 +748,11 @@ std::shared_ptr<decompress_factory> get_decompress_factory(const utility::string
return std::shared_ptr<decompress_factory>();
}


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
(void)compressionLevel;
(void)method;
Expand All @@ -760,11 +761,11 @@ std::unique_ptr<compress_provider> make_gzip_compressor(int compressionLevel, in
return std::unique_ptr<compress_provider>();
#endif // 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
(void)compressionLevel;
(void)method;
Expand All @@ -777,7 +778,7 @@ std::unique_ptr<compress_provider> make_deflate_compressor(int compressionLevel,
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
(void)window;
(void)quality;
Expand Down Expand Up @@ -962,7 +963,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 @@ -976,7 +977,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 @@ -1079,7 +1080,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 Down Expand Up @@ -1120,7 +1121,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