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

Fixed codec nullptr dereferencing. #264

Conversation

Hadatko
Copy link
Member

@Hadatko Hadatko commented Mar 29, 2022

Signed-off-by: Cervenka Dusan cervenka@acrios.com

Pull request

Choose Correct

  • bug
  • feature

Describe the pull request

#263

To Reproduce

Expected behavior

Screenshots

Desktop (please complete the following information):

  • OS:
  • eRPC Version:

Steps you didn't forgot to do

  • I checked if other PR isn't solving this issue.
  • I read Contribution details and did appropriate actions.
  • PR code is tested.
  • PR code is formatted.

Additional context

Signed-off-by: Cervenka Dusan <cervenka@acrios.com>
@Hadatko Hadatko self-assigned this Mar 29, 2022
@Hadatko
Copy link
Member Author

Hadatko commented Mar 29, 2022

This PR should be ready to merge.

@Hadatko Hadatko linked an issue Mar 29, 2022 that may be closed by this pull request
@Hadatko Hadatko added the bug label Mar 29, 2022
@Hadatko Hadatko added this to the 1.9.1 milestone Mar 29, 2022
@smithBraun
Copy link

I suggest (maybe in addition?) to call the dispose function just if it not null

    // Get a new request.
    RequestContext request = g_client->createRequest(false);

    // Encode the request.
    Codec * codec = request.getCodec();

    if (codec == NULL)
    {
        err = kErpcStatus_MemoryError;
    }
    else
    {
        ...
    // Dispose of the request.
    g_client->releaseRequest(request);
    }

@Hadatko
Copy link
Member Author

Hadatko commented Mar 30, 2022

I suggest (maybe in addition?) to call the dispose function just if it not null

    // Get a new request.
    RequestContext request = g_client->createRequest(false);

    // Encode the request.
    Codec * codec = request.getCodec();

    if (codec == NULL)
    {
        err = kErpcStatus_MemoryError;
    }
    else
    {
        ...
    // Dispose of the request.
    g_client->releaseRequest(request);
    }

I was thinking about this implementation too. It is correct, but it may be tricky if there will be change in RequestContext. Logically i don't like that createRequest is called in different scope than release request. As this are pair functions.

@smithBraun
Copy link

OK, agree.
Another thing that is bothering me her is that we know that g_client->createRequest(false); failed just with looking on the codec, isn't there is some status we can return and check it for failures?

@Hadatko
Copy link
Member Author

Hadatko commented Mar 30, 2022

OK, agree. Another thing that is bothering me her is that we know that g_client->createRequest(false); failed just with looking on the codec, isn't there is some status we can return and check it for failures?

I understand what you want to achieve. I think it is not necessary right now. But it can be usefull in future if another allocation may failed during RequestContext creation...

@MichalPrincNXP MichalPrincNXP merged commit bbcb60f into EmbeddedRPC:develop Mar 31, 2022
@MichalPrincNXP
Copy link
Member

Thank you!

@Hadatko Hadatko deleted the feature/fix_codec_nullptr_dereferencing branch June 8, 2022 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

NULL pointer dereference
3 participants