Skip to content

MSVC: Fix build errors and update CI for building - #296

Open
mrdeep1 wants to merge 1 commit into
eclipse-tinydtls:mainfrom
mrdeep1:msvc_builds
Open

mrdeep1 wants to merge 1 commit into
eclipse-tinydtls:mainfrom
mrdeep1:msvc_builds

Conversation

@mrdeep1

@mrdeep1 mrdeep1 commented Oct 3, 2026

Copy link
Copy Markdown
Contributor

There are a lot of size mismatch warnings that still need to be reviewed and fixed.

The tests have been updated to build with a MSVC environment.

Fixes #283.


steps:
- uses: actions/checkout@v6
- name: setup

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Having a MacOS-specific change in a PR that explicitly addresses MSVC is a bit weird.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fair comment - I will remove the Mac-OS CI run from building the tests and create a separate PR for it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually, because building unit-tests is introduced in this PR, cunit needs to be installed, or MAC (APPLE) needs to be explicitly removed from building testdriver in tests/CMakeLists.txt and then a new PR created specifically for MacOS.

Comment thread platform-specific/dtls_prng_win.c Outdated
*buf++ = number & 0xFF;
}
return klen;
return (int)klen;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since we are casting size_t (usually an unsigned type) to int I wonder if we should do a bit more hardening here. The compiler warning wanted to warn us about something, I suppose.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As can be seen in https://github.com/eclipse-tinydtls/tinydtls/actions/runs/37139114165/job/111249679547?pr=296 (expand out Build tinydtls) there are many implied casting issues which need to be resolved.

I just fixed the windows specific file one to align with the dtls_prng() definition as it is very unlikely, even on a 16 bit system, the be filing a buffer of 32768 bytes. Perhaps the better fix there would be to update the return from dtls_prng() to be the same as the passed in len parameter.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hm, this is interesting. The original intention was to have boolean result, so len == 0 would have done the trick. Now, this function has two exits: First is a return err when rand_s returns a non-zero result. AFAICT errno_t yields a positive value if an error occurs. So, dtls_prng() will indicate a "good" result even after the PRNG_FAILED message.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It makes sense to me that this is boolean (as indicated in dtls_prng.h), but all the other dtls_prng() variants return the filled in length to pass testdriver's ecc tests.

I will do a separate PR to make it all boolean for dtls_prng().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Separate PR created for dtls_prng(), and changes to dtls_prng() removed.

@mrdeep1

mrdeep1 commented Oct 5, 2026

Copy link
Copy Markdown
Contributor Author

Some of this work is duplicated, or done differently as in #263.

There are a lot of size mismatch warnings that still need to be reviewed
and fixed.

The tests have been updated to build with a MSVC environment.

Signed-off-by: Jon Shallow <supjps-libcoap@jpshallow.com>
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.

MSVC build is broken (undeclared DTLS_DEBUG_BUF_SIZE)

2 participants