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

Problem: message_t ctor string inconsistency #399

Merged
merged 2 commits into from
Sep 8, 2020

Conversation

gummif
Copy link
Member

@gummif gummif commented Apr 3, 2020

Solution: Constructor taking generic ranges including string literals includes the null terminated char in
the message. Deprecate function and add overloads for strings.

This fixes the root cause of a problem observed in #396 and discussed here #397

@sigiesec
Copy link
Member

Sorry I am lagging behind somewhat. I will try to give this and your other PRs a closer look in the next days. Is there one particularly urgent?

@gummif
Copy link
Member Author

gummif commented Apr 25, 2020

Alright, nothing urgent, just minor fixes.

@sigiesec
Copy link
Member

I still have some problems digesting this rather large PR. Unfortunately, the commits are not self-contained, as some of them fix issues from earlier commits. Is it possible to break this up into some self-contained steps, and rewrite the commits accordingly, in a way that would simplify review?

Is all of this really necessary to fix the issue from #396? (Not saying that the changes shouldn't be done, I am just trying to understand if there is one part fixing #396 and more things that are additional improvements)

@gummif
Copy link
Member Author

gummif commented May 16, 2020

I am going to split the unrelated changes into other PRs (see e.g. #416) before the rewrite.

@gummif gummif force-pushed the gfa/message-ctor branch 3 times, most recently from bf61392 to 7c9a79e Compare September 6, 2020 14:14
@gummif
Copy link
Member Author

gummif commented Sep 6, 2020

This has been simplified a lot and is ready for another review.

Copy link
Member

@sigiesec sigiesec left a comment

Choose a reason for hiding this comment

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

Thanks, now it was must easier to review this :)

zmq.hpp Outdated Show resolved Hide resolved
Solution: Constuctor taking generic ranges including
string literals includes the null terminated char in
the message. Deprecate function and add overloads
for strings.
@sigiesec sigiesec merged commit fef905d into zeromq:master Sep 8, 2020
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.

None yet

2 participants