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.

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

Fix unused variable for Win32 build in random_seed.c #700

Merged
merged 1 commit into from
Apr 15, 2021

Conversation

Philosoph228
Copy link
Contributor

CMake Release configuration add the -Werror flag, which fails the compilation if there any warning. There is an only one - unused variable.
Please let compilations succeed with Release configuration.

Build logs:

user@buildhost MINGW64 ~/json-c
$ mkdir build && cd $_

user@buildhost MINGW64 ~/json-c/build
$ cmake .. -G"MSYS Makefiles" -DCMAKE_BUILD_TYPE=Release
-- The C compiler identification is GNU 10.2.0
-- Check for working C compiler: C:/msys64/mingw64/bin/gcc.exe
-- Check for working C compiler: C:/msys64/mingw64/bin/gcc.exe - works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Looking for sys/resource.h
-- Looking for sys/resource.h - not found
-- Wrote C:/msys64/home/user/json-c/build/apps_config.h
-- Looking for fcntl.h
-- Looking for fcntl.h - found
-- Looking for inttypes.h
-- Looking for inttypes.h - found
-- Looking for stdarg.h
-- Looking for stdarg.h - found
-- Looking for strings.h
-- Looking for strings.h - found
-- Looking for string.h
-- Looking for string.h - found
-- Looking for syslog.h
-- Looking for syslog.h - not found
-- Looking for 4 include files stdlib.h, ..., float.h
-- Looking for 4 include files stdlib.h, ..., float.h - found
-- Looking for unistd.h
-- Looking for unistd.h - found
-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for dlfcn.h
-- Looking for dlfcn.h - not found
-- Looking for endian.h
-- Looking for endian.h - not found
-- Looking for limits.h
-- Looking for limits.h - found
-- Looking for locale.h
-- Looking for locale.h - found
-- Looking for memory.h
-- Looking for memory.h - found
-- Looking for stdint.h
-- Looking for stdint.h - found
-- Looking for stdlib.h
-- Looking for stdlib.h - found
-- Looking for sys/cdefs.h
-- Looking for sys/cdefs.h - found
-- Looking for sys/param.h
-- Looking for sys/param.h - found
-- Looking for sys/random.h
-- Looking for sys/random.h - not found
-- Looking for sys/stat.h
-- Looking for sys/stat.h - found
-- Looking for xlocale.h
-- Looking for xlocale.h - not found
-- Looking for _isnan
-- Looking for _isnan - found
-- Looking for _finite
-- Looking for _finite - found
-- Looking for INFINITY
-- Looking for INFINITY - found
-- Looking for isinf
-- Looking for isinf - found
-- Looking for isnan
-- Looking for isnan - found
-- Looking for nan
-- Looking for nan - found
-- Looking for _doprnt
-- Looking for _doprnt - not found
-- Looking for snprintf
-- Looking for snprintf - found
-- Looking for vasprintf
-- Looking for vasprintf - found
-- Looking for vsnprintf
-- Looking for vsnprintf - found
-- Looking for vprintf
-- Looking for vprintf - found
-- Looking for arc4random
-- Looking for arc4random - not found
-- Looking for bsd/stdlib.h
-- Looking for bsd/stdlib.h - not found
-- Looking for open
-- Looking for open - found
-- Looking for realloc
-- Looking for realloc - found
-- Looking for setlocale
-- Looking for setlocale - found
-- Looking for uselocale
-- Looking for uselocale - not found
-- Looking for strcasecmp
-- Looking for strcasecmp - found
-- Looking for strncasecmp
-- Looking for strncasecmp - found
-- Looking for strdup
-- Looking for strdup - found
-- Looking for strerror
-- Looking for strerror - found
-- Looking for strtoll
-- Looking for strtoll - found
-- Looking for strtoull
-- Looking for strtoull - found
-- Looking for stddef.h
-- Looking for stddef.h - found
-- Check size of int
-- Check size of int - done
-- Check size of int64_t
-- Check size of int64_t - done
-- Check size of long
-- Check size of long - done
-- Check size of long long
-- Check size of long long - done
-- Check size of size_t
-- Check size of size_t - done
-- Check size of ssize_t
-- Check size of ssize_t - done
-- Performing Test HAS_GNU_WARNING_LONG
-- Performing Test HAS_GNU_WARNING_LONG - Failed
-- Performing Test HAVE_ATOMIC_BUILTINS
-- Performing Test HAVE_ATOMIC_BUILTINS - Success
-- Performing Test HAVE___THREAD
-- Performing Test HAVE___THREAD - Success
-- Wrote C:/msys64/home/user/json-c/build/config.h
-- Wrote C:/msys64/home/user/json-c/build/json_config.h
-- Performing Test REENTRANT_WORKS
-- Performing Test REENTRANT_WORKS - Success
-- Performing Test BSYMBOLIC_WORKS
-- Performing Test BSYMBOLIC_WORKS - Success
-- Performing Test VERSION_SCRIPT_WORKS
-- Performing Test VERSION_SCRIPT_WORKS - Success
-- Could NOT find Doxygen (missing: DOXYGEN_EXECUTABLE)
Warning: doxygen not found, the 'doc' target will not be included
-- Configuring done
-- Generating done
-- Build files have been written to: C:/msys64/home/user/json-c/build

user@buildhost MINGW64 ~/json-c/build
$ cd ..

user@buildhost MINGW64 ~/json-c
$ cmake --build ./build --config Release
Scanning dependencies of target json-c
[  1%] Building C object CMakeFiles/json-c.dir/arraylist.c.obj
[  2%] Building C object CMakeFiles/json-c.dir/debug.c.obj
[  3%] Building C object CMakeFiles/json-c.dir/json_c_version.c.obj
[  4%] Building C object CMakeFiles/json-c.dir/json_object.c.obj
[  5%] Building C object CMakeFiles/json-c.dir/json_object_iterator.c.obj
[  7%] Building C object CMakeFiles/json-c.dir/json_pointer.c.obj
[  8%] Building C object CMakeFiles/json-c.dir/json_tokener.c.obj
[  9%] Building C object CMakeFiles/json-c.dir/json_util.c.obj
[ 10%] Building C object CMakeFiles/json-c.dir/json_visit.c.obj
[ 11%] Building C object CMakeFiles/json-c.dir/linkhash.c.obj
[ 13%] Building C object CMakeFiles/json-c.dir/printbuf.c.obj
[ 14%] Building C object CMakeFiles/json-c.dir/random_seed.c.obj
C:/msys64/home/user/json-c/random_seed.c: In function 'get_cryptgenrandom_seed':
C:/msys64/home/user/json-c/random_seed.c:274:6: error: unused variable 'r' [-Werror=unused-variable]
  274 |  int r;
      |      ^
cc1.exe: all warnings being treated as errors
make[2]: *** [CMakeFiles/json-c.dir/build.make:226: CMakeFiles/json-c.dir/random_seed.c.obj] Error 1
make[1]: *** [CMakeFiles/Makefile2:238: CMakeFiles/json-c.dir/all] Error 2
make: *** [Makefile:183: all] Error 2

Build Environment:

user@buildhost MINGW64 ~
$ uname -a
MINGW64_NT-10.0-19041 user@buildhost 3.1.7-340.x86_64 2020-11-08 12:32 UTC x86_64 Msys

user@buildhost MINGW64 ~
$ cmake --version
cmake version 3.17.3

CMake suite maintained and supported by Kitware (kitware.com/cmake).

user@buildhost MINGW64 ~
$ gcc --version
gcc.exe (Rev3, Built by MSYS2 project) 10.2.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

@coveralls
Copy link

coveralls commented Apr 12, 2021

Coverage Status

Coverage remained the same at 87.382% when pulling 9c05651 on Philosoph228:werror-fix into 041cef4 on json-c:master.

@ploxiln
Copy link
Contributor

ploxiln commented Apr 13, 2021

The change looks good to me. The PR and commit title could be more concise, maybe something like:

random_seed: fix unused variable for win32 build

It looks like this variable became unused (only for windows builds) in #674 and this project (understandably) only has CI windows tests using msvc (no gcc or clang on windows in CI).

@Philosoph228
Copy link
Contributor Author

I had opened the commit history and saw it's a really good idea. I'll amend the commit message, thank you.

@Philosoph228
Copy link
Contributor Author

Philosoph228 commented Apr 13, 2021

Um… I am not sure I did it right. Maybe squash changes? Or would I have to try from scratch?

@ploxiln
Copy link
Contributor

ploxiln commented Apr 13, 2021

No, you don't have to try from scratch. If you amend or rebase, you need to force push. If you just try to push after one of those operations, git will say "not a fast-forward merge, please merge in the remote branch first", so that's what you did, but it's not what you really wanted to do. You wanted to replace the remote state with the new version, rather than merging the new and old versions of your branch.

@ploxiln
Copy link
Contributor

ploxiln commented Apr 13, 2021

Looks like you figured it out :)

@Philosoph228 Philosoph228 changed the title Remove unused variable for CMake "Release" build type compliance Fix unused variable in random_seed.c for Win32 build Apr 13, 2021
@Philosoph228 Philosoph228 changed the title Fix unused variable in random_seed.c for Win32 build Fix unused variable for Win32 build in random_seed.c Apr 13, 2021
@hawicz hawicz merged commit 9490984 into json-c:master Apr 15, 2021
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.

None yet

4 participants