Fixes for Watcom compiler and new CI test#8391
Conversation
|
Below is Github Action definition file modified to automate any host and any target with Open Watcom toolchain by matrix. |
Wonderful! I'll give this a try. Thank you! |
JacobBarthelmeh
left a comment
There was a problem hiding this comment.
Please resolve merge conflict
|
Hi @dgarske, I have prepared regular fixes for Open Watcom against current version of WolfSSL repository that All build of WolfSSL is going OK for Windows multi-threaded version (32-bit). Thanks Jiri |
|
Hi Jiri, We'd love to get it working with the C89 compliance level, since we have customers using that even with Watcom C. I am about to rebase this PR and force push to resolve a merge conflict. You are welcome to share a patch and I can add it. OR Thanks, |
|
OK I will separate fixes relate to C89, it is mainly initilization by non-constant values and sometimes definition of variables in code. |
In your PR I recommend trying to remove the |
|
OK I will try.
Open Watcom fixes requires C89 fixes and winsock too or some alternative solution Build tested with Open Watcom 1.9 and 2.0 !! it is fixes only for Windows target !! There is some duplication code for thread support, but it is prepared for OS/2 build changes. Now it is same as for Microsoft compilers with small differences. |
|
Sorry, one mistake. |
|
@jmalak let me know if you have suggestions to fix: |
|
I am running CMake by command cmake -B build -G "Watcom WMake" -D CMAKE_VERBOSE_MAKEFILE=TRUE -D CMAKE_SYSTEM_NAME=Windows -D CMAKE_SYSTEM_PROCESSOR=x86 -D WOLFSSL_ASM=no -DCMAKE_BUILD_TYPE=Release I will check with your setup, it looks like some mistake if single-thread configuration is used, I didn't check I am building multi-threaded version only on Windows |
|
You should fix wc_port.h to contains windows.h for both multi and single threaded build |
|
I got following results on Windows now and benchmarks It is pure C code without assembler code. |
|
I am in trouble with Linux target. Why WolfSSL touch |
|
I did correction to your watcomc.yml to synchronize OW tools options with WolfSSL macros setup which is incorrect. |
|
Hi David, I am in trouble with two general issues.
first it is same as C standard macro Is it possible to do these changes, I don't know reason why it was done this way, maybe on some platforms is necessary for some reason. |
|
here is latest patch Regards Jiri |
|
I enclosed fix for OFFSETOF macro issue which I am using |
I am trying to avoid this refactor from OFFSETOF -> offsetof. My patching attempts have failed and still getting: Can you explain why this patch doesn't work? I used the same OS2 offsetof macro in stddef.h. |
|
Problem is multiple definition of macro OFFSETOF and current definition OFFSETOF in WolfSSL But WolfSSL definition is always before OS/2 headers that it report different definition. Problem is that I cannot rename Operating system definition of OFFSETOF to different name that only solution is rename it in WolfSSL because it is luckily internal macro and also it is same as C standard macro offsetof (stddef.h) which should be used instead OFFSETOF. offsetof macro must be defined in any toolchain because it is part of C from K&R epoch. |
|
by example gcc 11 and 12 has it defined on Linux as #define offsetof(TYPE, MEMBER) __builtin_offsetof (TYPE, MEMBER) in OW it is #define offsetof(__typ,__id) ((size_t)((char )&(((__typ)0)->__id) - (char *)0)) every toolchain has own definition which cover defined function offset of member in structure as described in C standard. Visual studio 2022 #if defined _MSC_VER && !defined _CRT_USE_BUILTIN_OFFSETOF |
* Correct cmake script to support Open Watcom toolchain (wolfSSL#8167) * Fix thread start callback prototype for Open Watcom toolchain (wolfSSL#8175) * Added GitHub CI action for Windows/Linux/OS2 * Improvements for C89 compliance. Thank you @jmalak for your contributions.
|
I enclosed new OFFSETOF patch, which could be acceptable. |
This could work, but I'll need to make sure there aren't any customers overriding |
|
I add also support for Open Watcom 1.9 and add apropriate tests. |
Very nice work! I'll need a bit of time to integrate these changes. Thank you again @jmalak |
Well the PR got merged in. If you want to open a followup PR with those proposed changes, feel free to. I hadn't updated the PR with your patch. I think the OFFSETOF changes will be okay. |
|
OK I will include it with other OS/2 changes when I will finish (still not finished implementation), because it is showstopper for OS/2 version only not for other targets. Regards Jiri |
Description
Fixes for Watcom compiler and new CI test
Thank you @jmalak for your contributions.
Testing
See new CI test
Checklist