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

Fix mismatched macro guard between c and header file #157

Conversation

lhuang04
Copy link
Collaborator

@lhuang04 lhuang04 commented Mar 8, 2021

Summary:
The following functions are under different macro guard between c file and header file

  • mbedtls_ssl_set_session
  • mbedtls_ssl_session_save
  • mbedtls_ssl_session_load
  • mbedtls_ssl_get_session_pointer

https://github.com/hannestschofenig/mbedtls/blob/tls13-prototype/programs/ssl/ssl_client2.c#L3488
is used in client, so the
MBEDTLS_SSL_SRV_C macro in C file seems incorrect.
Change the macro in c file to match the header file.

Also move ssl_serialized_session_header inside as it is only used in
the block, and don't need different macro guard

Test Plan:

Undefine MBEDTLS_SSL_SRV_C, and make

git diff
diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h
index 80a632613..1a4370ef5 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -3386,7 +3386,7 @@
  *
    * This module is required for SSL/TLS server support.
      */
      -#define MBEDTLS_SSL_SRV_C
      +//#define MBEDTLS_SSL_SRV_C

       /**
           * \def MBEDTLS_SSL_TLS_C

The following compiler error should be fixed.

/home/lhuang04/upstream/library/ssl_tls.c:5928:22: error:
‘ssl_serialized_session_header’ defined but not used
[-Werror=unused-variable]
 static unsigned char ssl_serialized_session_header[] = {
                       ^
                       cc1: all warnings being treated as errors
                       make[2]: ***
                       [library/CMakeFiles/mbedtls.dir/ssl_tls.c.o]
                       Error 1
                       make[2]: *** Waiting for unfinished jobs....
                       [ 34%] Built target cert_write
                       make[1]: *** [library/CMakeFiles/mbedtls.dir/all]
                       Error 2
                       make: *** [all] Error 2

Reviewers:

Subscribers:

Tasks:

Tags:

Summary:
The following functions are under different macro guard between [c
file](https://github.com/hannestschofenig/mbedtls/blob/tls13-prototype/library/ssl_tls.c#L5937) and
[header
file](https://github.com/hannestschofenig/mbedtls/blob/tls13-prototype/include/mbedtls/ssl.h#L2950-L2951)

* mbedtls_ssl_set_session
* mbedtls_ssl_session_save
* mbedtls_ssl_session_load
* mbedtls_ssl_get_session_pointer

[https://github.com/hannestschofenig/mbedtls/blob/tls13-prototype/programs/ssl/ssl_client2.c#L3488](https://github.com/hannestschofenig/mbedtls/blob/tls13-prototype/programs/ssl/ssl_client2.c#L3488)
is used in client, so the
[MBEDTLS_SSL_SRV_C](https://github.com/hannestschofenig/mbedtls/blob/tls13-prototype/library/ssl_tls.c#L5937) macro in C file seems incorrect.
Change the macro in c file to match the header file.

Also move `ssl_serialized_session_header` [inside](https://github.com/hannestschofenig/mbedtls/blob/tls13-prototype/library/ssl_tls.c#L5937) as it is only used in
the block, and don't need different macro guard

Test Plan:

Undefine MBEDTLS_SSL_SRV_C, and make
```
git diff
diff --git a/include/mbedtls/config.h b/include/mbedtls/config.h
index 80a632613..1a4370ef5 100644
--- a/include/mbedtls/config.h
+++ b/include/mbedtls/config.h
@@ -3386,7 +3386,7 @@
  *
    * This module is required for SSL/TLS server support.
      */
      -#define MBEDTLS_SSL_SRV_C
      +//#define MBEDTLS_SSL_SRV_C

       /**
           * \def MBEDTLS_SSL_TLS_C
```

The following compiler error should be fixed.
```
/home/lhuang04/upstream/library/ssl_tls.c:5928:22: error:
‘ssl_serialized_session_header’ defined but not used
[-Werror=unused-variable]
 static unsigned char ssl_serialized_session_header[] = {
                       ^
                       cc1: all warnings being treated as errors
                       make[2]: ***
                       [library/CMakeFiles/mbedtls.dir/ssl_tls.c.o]
                       Error 1
                       make[2]: *** Waiting for unfinished jobs....
                       [ 34%] Built target cert_write
                       make[1]: *** [library/CMakeFiles/mbedtls.dir/all]
                       Error 2
                       make: *** [all] Error 2

```

Reviewers:

Subscribers:

Tasks:

Tags:
@hannestschofenig
Copy link
Owner

I looked at this PR and tested it. Once it is merged I would like to add a small addition to allow compilation of the code without any ticket handling support.

In any case, I prefer this to get merged soon.

@hannestschofenig hannestschofenig merged commit 62e6356 into hannestschofenig:tls13-prototype Mar 15, 2021
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.

2 participants