mattgodbolt / seasocks

Simple, small, C++ embeddable webserver with WebSockets support
BSD 2-Clause "Simplified" License
734 stars 120 forks source link

MD5: Removed null pointer subtraction #171

Closed rayburgemeestre closed 2 years ago

rayburgemeestre commented 2 years ago

Hi all,

Thanks for this great project!

Created this PR after initial discussion here: https://github.com/mattgodbolt/seasocks/pull/165

My environment was Ubuntu 22.04, with the system clang14 package (Ubuntu clang version 14.0.0-1ubuntu1)

Before this change, this is the compile error:

/home/trigen/projects/build-config/seasocks/seasocks/src/main/c/md5/md5.cpp:190:25: error: performing pointer subtraction with a null pointer may have undefined behavior [-Werror,-Wnull-pointer-subtraction]
            if (!((data - (const md5_byte_t*) 0) & 3)) {
                        ^ ~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [src/main/c/CMakeFiles/seasocks.dir/build.make:146: src/main/c/CMakeFiles/seasocks.dir/md5/md5.cpp.o] Error 1

I researched it a little bit, found the PR I commented on previously, and further googling found a fix made by Maxim Sharabayko in a different project: https://github.com/Haivision/srt/pull/2392 The fix made sense to me, tried it, and also solved the compilation issue for me.

Cheers, Ray

codecov[bot] commented 2 years ago

Codecov Report

Merging #171 (d79eaa3) into master (4f4e64d) will not change coverage. The diff coverage is 0.00%.

@@           Coverage Diff           @@
##           master     #171   +/-   ##
=======================================
  Coverage   35.88%   35.88%           
=======================================
  Files          53       53           
  Lines        2302     2302           
=======================================
  Hits          826      826           
  Misses       1476     1476           
Impacted Files Coverage Δ
src/main/c/md5/md5.cpp 0.00% <0.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

offa commented 2 years ago

@mattgodbolt could you have a look at it too? It works on latest clang and Gcc.