c89atomic icon indicating copy to clipboard operation
c89atomic copied to clipboard

added GitHub Action CI & tests for atomicity

Open TheTechsTech opened this issue 1 year ago • 1 comments

  • modifications with additional macros for naming compatibility
  • added generic cmakelists.txt build file

TheTechsTech avatar Jul 10 '24 17:07 TheTechsTech

Thanks. A proper test for atomicity is a valuable contribution, and something badly lacking for this library. However, I cannot merge this change in its current state. For starters all whitespace and styling changes need to be reverted (including the position of curly braces). After that I can give it a proper review.

Regarding the CMake file, I'm happy enough to add this, however I noticed that you've disabled some warnings:

if(UNIX)
    set(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -g")
    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -O3 -fomit-frame-pointer -Wno-return-type")
else()
    add_definitions("/wd4244 /wd4267 /wd4033 /wd4715")
endif()

Are these warnings from c89atomic.h, or the tests? Would you be able to show me a list so I can fix them properly? If they're from your new tests I'd rather they just be fixed naturally in the code than just silenced through the build system.

Another thing, and this is not something that would stop me from merging, but for the thread.c/h files, my preference would be to just use my c89thread library. The reason I say that is just I'm just so tired of maintaining cross-platform threading stuff and I'd rather just use an easy-to-integrate library instead. That's not a major issue though - I can change that later.

mackron avatar Jul 12 '24 00:07 mackron

Haven't been keeping up with changes, I'm closing this. I'm currently doing stress testing, things not as expected on my changes.

Seems your resent updates fixes some concerns, added CMakeLists.txt build, and actual do thread atomic testing, pretty good!

TheTechsTech avatar Oct 15 '25 19:10 TheTechsTech