Add support for const error description strings

[aggarg]

Problem

mbedtls_strerror is a utility function which converts an mbedTLS error code into a human readable string. It requires the caller to allocate a buffer every time an error code needs to be converted to a string. It is an overkill and a waste of RAM for resource constrained microcontrollers - where the most common use case is to use these strings for logging.

Solution

The proposed commit adds two functions:

const char * mbedtls_high_level_strerr( int error_code );
const char * mbedtls_low_level_strerr( int error_code );

The above two functions convert the high level and low level parts of an mbedTLS error code to human readable strings. They return a const pointer to an unmodifiable string which is not supposed to be modified by the caller and only to be used for logging purposes. The caller no longer needs to allocate a buffer.

Status

READY

Requires Backporting

NO

The proposed change is completely backward compatible as it does not change the existing mbedtls_strerror function and ensures that it continues to behave the same way.

Files

Modified Files:

• include/mbedtls/error.h
• scripts/data_files/error.fmt
• scripts/generate_errors.pl

Auto Generated File:

• library/error.c

The library/error.c is generated by running scripts/generate_errors.pl (which seems to be the way it is generated).

Testing

./test_suite_error 

Single low error .................................................. PASS
Single high error ................................................. PASS
Low and high error ................................................ PASS
Non existing high error ........................................... PASS
Non existing low error ............................................ PASS
Non existing low and high error ................................... PASS

----------------------------------------------------------------------------

PASSED (6 / 6 tests (0 skipped))

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

[aggarg]

I do not have access to the internal jenkins server. Can someone help me with what the error in the failed PR checks is?

[gilles-peskine-arm]

Everything that should have passed has passed. The Mbed OS testing is currently broken, but that doesn't hold up PRs.

[aggarg]

Everything that should have passed has passed. The Mbed OS testing is currently broken, but that doesn't hold up PRs.

Thank you. Would you please help me in getting the required approvals then?

[gilles-peskine-arm]

It's on our review dashboard now.

[aggarg]

It's on our review dashboard now.

Thank you.

[aggarg]

Ping - Request for review.

[gilles-peskine-arm]

I'm ok with the design, except that I wonder if mbedtls_high_level_strerr and mbedtls_low_level_strerr should operate on the high/low part of the error code, rather than require it to solely consist of a high/low error code.

I'm requesting some minor documentation improvements. Comments on mbedtls_high_level_strerr also apply to mbedtls_low_level_strerr.

[aggarg]

I'm ok with the design, except that I wonder if mbedtls_high_level_strerr and mbedtls_low_level_strerr should operate on the high/low part of the error code, rather than require it to solely consist of a high/low error code.

I'm requesting some minor documentation improvements. Comments on mbedtls_high_level_strerr also apply to mbedtls_low_level_strerr.

Thank you for the review. Yes, I think it is a good suggestion - I have updated mbedtls_high_level_strerr and mbedtls_low_level_strerr to accept an error code and extract the high-level and low-level parts respectively. Also, made all the suggested documentation updates.

[gilles-peskine-arm]

Thanks for the update! The CI failures are due to a temporary problem on the development branch (a file that broke check-files.py and check-generated-files.sh) which is now fixed. The failures don't prevent getting relevant results for this PR and the CI can be considered passing.

[irwir]

@gilles-peskine-arm
So finally someone was found who converted the scripts to generate switch statements, as was discussed long ago. :)

One additional remark.
It appear that error_description variable is not needed at all, and the code easily could be made even more concise as in the following example:

        case -(MBEDTLS_ERR_XTEA_INVALID_INPUT_LENGTH):
            return( "XTEA - The data input has an invalid length" );
        case ...

Should there be no objections, I guess @aggarg could do this. Would you?

[aggarg]

@gilles-peskine-arm
So finally someone was found who converted the scripts to generate switch statements, as was discussed long ago. :)

One additional remark.
It appear that error_description variable is not needed at all, and the code easily could be made even more concise as in the following example:

        case -(MBEDTLS_ERR_XTEA_INVALID_INPUT_LENGTH):
            return( "XTEA - The data input has an invalid length" );
        case ...

Should there be no objections, I guess @aggarg could do this. Would you?

Sure, will do. I am habitual of writing functions with single exit point which is why this style.