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

Some patch suggestions #53

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Some patch suggestions #53

wants to merge 3 commits into from

Conversation

micmac1
Copy link

@micmac1 micmac1 commented Oct 6, 2019

Hi all,

Some suggestions after toying around with libks.

One other thing, I think the cmake files should probably not be installed in /usr/include/libks/cmake. I think "$LIBDIR"/cmake/libks is probably correct. But then you'd also need to update the users, like signalwire-c.

Regards,
Seb

P.S.: Feel free to close this if you would like to solve this some other way.

Currently the search paths for math.h are set statically. This results
in being unable to compile libks in a buildroot when cross-compiling.

<snip>
-- Using unsigned short
-- Check if the system is big endian - big endian
-- Could NOT find LibM (missing: LIBM_INCLUDE_DIRS)
<snip>
-- Configuring done
CMake Error at CMakeLists.txt:332 (add_library):
  Target "ks" links to target "LIBM::LIBM" but the target was not found.
  Perhaps a find_package() call is missing for an IMPORTED target, or an
  ALIAS target is missing?
<snip>

Fix this by allowing standard search paths.

Signed-off-by: Sebastian Kemper <[email protected]>
The include should read <signal.h> instead of <sys/signal.h>. Some
toolchains show a warning when the include is wrong:

In file included from /home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/src/include/libks/ks_platform.h:94:0,
                 from /home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/src/include/libks/ks.h:32,
                 from /home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/src/ks.c:23:
/home/sk/tmp/openwrt/staging_dir/toolchain-mips_24kc_gcc-7.4.0_musl/include/sys/signal.h:1:2: warning: #warning redirecting incorrect #include <sys/signal.h> to <signal.h> [-Wcpp]
 #warning redirecting incorrect #include <sys/signal.h> to <signal.h>
  ^~~~~~~

This commit fixes the include.

Signed-off-by: Sebastian Kemper <[email protected]>
Currently ksutil_setup_platform() is called before project(). That's why
distro (or user) compiler flags are not used. When you want to make
amendments to the flags you need to do so _after_ project().

[  4%] Building C precompiled header cotire/ks_C_prefix.h.gch
cc1: note: someone does not honour COPTS correctly, passed 0 times
Scanning dependencies of target ks
make[5]: Leaving directory '/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6'
make[5]: Entering directory '/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6'
[  5%] Building C object CMakeFiles/ks.dir/src/include/cJSON/cJSON.c.o
cc1: note: someone does not honour COPTS correctly, passed 0 times
[  7%] Building C object CMakeFiles/ks.dir/src/include/cJSON/cJSON_Utils.c.o
cc1: note: someone does not honour COPTS correctly, passed 0 times

More explicit example:

/home/sk/tmp/openwrt/staging_dir/toolchain-mips_24kc_gcc-7.4.0_musl/bin/mips-openwrt-linux-musl-gcc -DHAVE_BYTESWAP_H=1 -DHAVE_CLOCK_GETRES=1 -DHAVE_CLOCK_GETTIME=1 -DHAVE_CLOCK_NANOSLEEP=1 -DHAVE_DIRENT_H=1 -DHAVE_DLFCN_H=1 -DHAVE_INTTYPES_H=1 -DHAVE_LIBCRYPTO=1 -DHAVE_LIBSSL=1 -DHAVE_MALLOC=1 -DHAVE_MEMMEM=1 -DHAVE_MEMORY_H=1 -DHAVE_PTHREAD_SETSCHEDPARAM=1 -DHAVE_SCHED_H=1 -DHAVE_SCHED_SETSCHEDULER=1 -DHAVE_STDINT_H=1 -DHAVE_STDLIB_H=1 -DHAVE_STRFTIME=1 -DHAVE_STRINGS_H=1 -DHAVE_STRING_H=1 -DHAVE_SYS_STAT_H=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_UNISTD_H=1 -DHAVE_USLEEP=1 -DKS_PLAT_LIN=1 -DRETSIGTYPE=void -DSTDC_HEADERS=1 -DTIME_WITH_SYS_TIME=1 -D_REENTRANT=1 -D__BYTE_ORDER=__BIG_ENDIAN -I/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/tests -I/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/src/include -I/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6 -std=c11 -DNDEBUG -fPIE -O2 -g -std=gnu99 -o CMakeFiles/testq.dir/tap.c.o -c /home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6/tests/tap.c

This was seen while

CMAKE_C_FLAGS:STRING=-Os -pipe -mno-branch-likely -mips32r2 -mtune=24kc -fno-caller-saves -fno-plt -fhonour-copts -Wno-error=unused-but-set-variable -Wno-error=unused-result -msoft-float -mips16 -minterlink-mips16 -iremap/home/sk/tmp/openwrt/build_dir/target-mips_24kc_musl/libks-2019-09-18-df72c4c6:libks-2019-09-18-df72c4c6 -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -I/home/sk/tmp/openwrt/staging_dir/target-mips_24kc_musl/usr/include -I/home/sk/tmp/openwrt/staging_dir/target-mips_24kc_musl/include -I/home/sk/tmp/openwrt/staging_dir/toolchain-mips_24kc_gcc-7.4.0_musl/usr/include -I/home/sk/tmp/openwrt/staging_dir/toolchain-mips_24kc_gcc-7.4.0_musl/include/fortify -I/home/sk/tmp/openwrt/staging_dir/toolchain-mips_24kc_gcc-7.4.0_musl/include

was set in CMakeCache.txt (distro flags). So none of the distro flags were used.

Fix this by rearanging rearranging CMakeLists.txt a bit.

Signed-off-by: Sebastian Kemper <[email protected]>
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.

1 participant