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

check null pointer before using memcpy() #891

Merged
merged 1 commit into from
Jul 2, 2020

Conversation

ygj6
Copy link
Collaborator

@ygj6 ygj6 commented Jun 30, 2020

Avoid passing null pointer to std::memcpy().
Fixed the following error mentioned by @jamessan in issue #881:

runtime error: null pointer passed as argument 2, which is declared to never be null

std::memcpy(m_data + m_size, buf, len);
if(buf) {
std::memcpy(m_data + m_size, buf, len);
}
m_size += len;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this only be inside the if?

Copy link
Contributor

Choose a reason for hiding this comment

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

same as #890 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

@ygj6 , could you apply the same fix as #890 ?

@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2020

Codecov Report

Merging #891 into cpp_master will decrease coverage by 0.07%.
The diff coverage is 100.00%.

@@              Coverage Diff               @@
##           cpp_master     #891      +/-   ##
==============================================
- Coverage       86.51%   86.44%   -0.08%     
==============================================
  Files              72       72              
  Lines            5071     5089      +18     
==============================================
+ Hits             4387     4399      +12     
- Misses            684      690       +6     

@redboltz
Copy link
Contributor

redboltz commented Jul 1, 2020

It seems that CI errors are reported.

I think that visit_str, visit_bin, and visit_ext should be as follows:

    bool visit_str(const char* v, uint32_t size) {
        assert(v || size == 0);
        if (size > m_limit.str()) throw msgpack::str_size_overflow("str size overflow");
        msgpack::object* obj = m_stack.back();
        obj->type = msgpack::type::STR;
        if (m_func && m_func(obj->type, size, m_user_data)) {
            obj->via.str.ptr = v;
            obj->via.str.size = size;
            set_referenced(true);
        }
        else {
            if (v) {
                char* tmp = static_cast<char*>(zone().allocate_align(size, MSGPACK_ZONE_ALIGNOF(char)));
                std::memcpy(tmp, v, size);
                obj->via.str.ptr = tmp;
                obj->via.str.size = size;
            }
            else {
                obj->via.str.ptr = MSGPACK_NULLPTR;
                obj->via.str.size = 0;
            }
        }
        return true;
    }

@redboltz
Copy link
Contributor

redboltz commented Jul 2, 2020

I'm not sure #891 (comment) solves the CI errors.
I tried to reproduce the error on my environment but I couldn't, so far.

@redboltz
Copy link
Contributor

redboltz commented Jul 2, 2020

I'm not sure #891 (comment) solves the CI errors.
I tried to reproduce the error on my environment but I couldn't, so far.

I've tested my update and it works well.
So I apply the update and merge the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants