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.

Sign up for GitHub

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

mbed TLS 1.3: Fix compilation warnings with VS2015 x64 #999

Closed
wants to merge 8 commits into from
Closed

mbed TLS 1.3: Fix compilation warnings with VS2015 x64 #999

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
83081c5
Fix MSVC warning in sample programs
Jul 6, 2017
2a0344f
Fix MSVC warning in net.c
Jul 6, 2017
c4b659c
Fix type in net.c comment
Jul 6, 2017
e8c587e
Add brackets around function macro arguments
Jul 6, 2017
5ad6be7
Add brackets around net.c macro arguments
Jul 12, 2017
28d7d3a
Change param type for x509_parse_int() to fix warn
Jul 31, 2017
e4fd1ba
Fix coding style in x509_parse_int()
Jul 31, 2017
eb01d3c
Add ChangeLog entry
Jul 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions library/net.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,14 @@ static int wsa_init_done = 0;

#endif /* ( _WIN32 || _WIN32_WCE ) && !EFIX64 && !EFI32 */

/* Some MS functions want int and MSVC warns if we pass size_t,
* but the standard functions use socklen_t, so cast only for MSVC */
#if defined(_MSC_VER)
Copy link

Choose a reason for hiding this comment

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

There's a typo here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied that from the development branch, so the typo comes from there. I will fix the typo here but I do not think its worth creating a PR for development and mbed TLS 2.1.

Copy link

Choose a reason for hiding this comment

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

Yes, I just saw that. Agreed, we can fix the typo in 2.1 and development on other occasions.

#define MSVC_INT_CAST (int)
#else
#define MSVC_INT_CAST
#endif

#include <stdlib.h>
#include <stdio.h>

Expand Down Expand Up @@ -202,7 +210,7 @@ int net_connect( int *fd, const char *host, int port )
continue;
}

if( connect( *fd, cur->ai_addr, cur->ai_addrlen ) == 0 )
if( connect( *fd, cur->ai_addr, MSVC_INT_CAST cur->ai_addrlen ) == 0 )
{
ret = 0;
break;
Expand Down Expand Up @@ -299,7 +307,7 @@ int net_bind( int *fd, const char *bind_ip, int port )
continue;
}

if( bind( *fd, cur->ai_addr, cur->ai_addrlen ) != 0 )
if( bind( *fd, cur->ai_addr, MSVC_INT_CAST cur->ai_addrlen ) != 0 )
{
close( *fd );
ret = POLARSSL_ERR_NET_BIND_FAILED;
Expand Down
2 changes: 1 addition & 1 deletion programs/ssl/ssl_client2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1267,7 +1267,7 @@ int main( int argc, char *argv[] )

len = polarssl_snprintf( (char *) buf, sizeof(buf) - 1, GET_REQUEST,
opt.request_page );
tail_len = strlen( GET_REQUEST_END );
tail_len = (int) strlen( GET_REQUEST_END );
Copy link
Contributor

Choose a reason for hiding this comment

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

we should consider changing tail_len to size_t

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I looked at the code again and I think that if we change this to size_t we will eventually need to use casts back to int. The problem is that functions such as snprintf() return int. Also, note that all the input parameters, etc are of type int so its not that straight forward to change one type. I think its best if we just investigate these problems as part of #1007


/* Add padding to GET request to reach opt.request_size in length */
if( opt.request_size != DFL_REQUEST_SIZE &&
Expand Down
4 changes: 2 additions & 2 deletions programs/ssl/ssl_mail_client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
#include <unistd.h>
#else
#include <io.h>
#define read _read
#define write _write
#define read(fd, buf, len) _read( fd, (void *)buf, (unsigned int)len )
#define write(fd, buf, len) _write( fd, (const void *)buf, (unsigned int)len )
#endif
Copy link

Choose a reason for hiding this comment

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

Could you add brackets around the arguments to ensure the casts also work as intended if compound expressions are passed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Again, I copied this from the development branch: https://github.com/ARMmbed/mbedtls/blob/development/library/net_sockets.c#L66


#if defined(_WIN32) || defined(_WIN32_WCE)
Expand Down
2 changes: 1 addition & 1 deletion programs/ssl/ssl_server2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1791,7 +1791,7 @@ int main( int argc, char *argv[] )
unsigned char *larger_buf;

ori_len = ret;
extra_len = ssl_get_bytes_avail( &ssl );
extra_len = (int) ssl_get_bytes_avail( &ssl );
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is better to change extra_len to size_t type, and thus remove the need for casting

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that is a good idea, however everything I have done here is backport the fix from the development branch. Do you think I should try to fix the problem here or raise another issue to address this in all the 3 branches?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think both:)
I think you should fix here, as we are already modifying the file, and raise another issuer for the other two branches

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RonEld: Even if we change this to size_t we will eventually need to cast as the return type of ssl_read() is int. In any case, we will not overflow the buffer because according to the RFC 5246 the length of the TLS record's content cannot exceed 2^14.


larger_buf = polarssl_malloc( ori_len + extra_len + 1 );
if( larger_buf == NULL )
Expand Down