Skip to content

ICC fixes #738

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

Merged
merged 2 commits into from
Mar 1, 2017
Merged

ICC fixes #738

merged 2 commits into from
Mar 1, 2017

Conversation

eile
Copy link

@eile eile commented Mar 1, 2017

const-qualify function-local static constant arrays.

@@ -131,7 +131,7 @@ class basic_client_facade {
} else {
if (boost::empty(content_type_headers)) {
typedef typename char_<Tag>::type char_type;
static char_type content_type[] = "x-application/octet-stream";
static char_type* content_type = "x-application/octet-stream";
Copy link
Member

Choose a reason for hiding this comment

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

I suppose a better change here would be:

static char_type const content_type[] = "...";

@eile
Copy link
Author

eile commented Mar 1, 2017

Updated.

@deanberris
Copy link
Member

LGTM -- thanks @eile!

@deanberris deanberris merged commit f17d305 into cpp-netlib:0.13-release Mar 1, 2017
@eile eile mentioned this pull request Mar 1, 2017
@tribal-tec
Copy link

@deanberris your requested change actually does not work, at least with icc 17:

boost/network/protocol/http/client/facade.hpp(134): error: a value of type "const char [27]" cannot be used to initialize an entity of type "const char_type []"
          static char_type const content_type[] = "x-application/octet-stream";

I suggest to keep the initial code change by @eile:

static char_type* const content_type = "x-application/octet-stream";

I just verified that this compiles.

@deanberris
Copy link
Member

@tribal-tec -- that's interesting, why isn't this an ICC bug?

If you send me a pull request I'd be happy to review and merge it.

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.

3 participants