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

Remove error_description variable from strerr functions #3219

Merged
merged 2 commits into from
May 4, 2020

Conversation

aggarg
Copy link
Contributor

@aggarg aggarg commented Apr 21, 2020

Description

The definition of the newly added functions mbedtls_high_level_strerr and mbedtls_low_level_strerr were of the form:

const char * mbedtls_high_level_strerr( int error_code )
{
    const char * error_description = NULL;

    switch( error_code )
    {
        case -(MBEDTLS_ERR_X509_FILE_IO_ERROR):
            error_description = "X509 - Read/write of file failed";
            break;

        default:
            break;
    }

    return error_description;
}

As suggested on this PR: #3176, we can remove the use of error_description variable:

const char * mbedtls_high_level_strerr( int error_code )
{
    switch( error_code )
    {
        case -(MBEDTLS_ERR_X509_FILE_IO_ERROR):
            return "X509 - Read/write of file failed";

        default:
            break;
    }

    return NULL;
}

This PR implements the suggestion.

Status

READY

Requires Backporting

NO

Signed-off-by: Gaurav Aggarwal [email protected]

This was suggested on this PR: Mbed-TLS#3176

Signed-off-by: Gaurav Aggarwal <[email protected]>
@irwir
Copy link
Contributor

irwir commented Apr 22, 2020

Looks good, but one change would be requested by reviewers - additional parentheses with spaces around returned value as said in local coding standards description.
In your example It should have been:
return( "X509 - Read/write of file failed" );
return( NULL );.

@aggarg
Copy link
Contributor Author

aggarg commented Apr 22, 2020

Looks good, but one change would be requested by reviewers - additional parentheses with spaces around returned value as said in local coding standards description.
In your example It should have been:
return( "X509 - Read/write of file failed" );
return( NULL );.

Good point. Will address.

return statements use parentheses to contain their value.

Signed-off-by: Gaurav Aggarwal <[email protected]>
@gilles-peskine-arm gilles-peskine-arm added component-platform Portability layer and build scripts enhancement needs-review Every commit must be reviewed by at least two team members, labels Apr 22, 2020
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

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

Thanks for this contribution, LGTM. 2 approvals and the CI is ok thus labelling this PR as ready for merge.

@ronald-cron-arm ronald-cron-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, labels May 4, 2020
@gilles-peskine-arm gilles-peskine-arm merged commit 9515d76 into Mbed-TLS:development May 4, 2020
@aggarg aggarg deleted the err_optimization branch May 4, 2020 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports component-platform Portability layer and build scripts enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants