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

Update autoconf M4 files #71

Merged
merged 8 commits into from
May 23, 2018
Merged

Update autoconf M4 files #71

merged 8 commits into from
May 23, 2018

Conversation

dgarske
Copy link
Contributor

@dgarske dgarske commented May 21, 2018

  • Update several of the macro files paralleling wolfSSL's m4 files.
  • Fix for ./configure --enable-debug=verbose to also set the debug flags.
  • Fix for CFLAGS.

* Update several of the macro files paralleling wolfSSL's m4 files.
* Force the `AR_FLAGS` to be `cr` in `configure.ac`.
* Add the `-Xcompiler` flag ahead of `-Qunused-arguments` for `-pthreads`.
…ion of configuration options in `wolfmqtt/options.h`. Cleanup of the configure.ac to use AM_CFLAGS and remove AX_PTHREAD.
configure.ac Outdated
@@ -34,6 +38,11 @@ WOLFMQTT_LIBRARY_VERSION=2:0:0
# +- increment if interfaces have been added, removed or changed
AC_SUBST([WOLFMQTT_LIBRARY_VERSION])

# capture user C_EXTRA_FLAGS from ./configure line, CFLAGS may hold -g -O2 even
# if user doesn't override, no way to tell
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Up on line 11 and line 14, we know why CFLAGS gets default values. It is due to AC_PROG_CC. Line 11 is the idiom for initializing a shell variable when the caller doesn't pass one in. AC_PROG_CC won't add "-g -O2" because of that, then we tack in on later as appropriate.

… Fix to hush ar warning (adds the "U" option to the ar flags if the version of ar supports it).
ejohnstown
ejohnstown previously approved these changes May 23, 2018
configure.ac Outdated
@@ -11,7 +11,10 @@ AC_CONFIG_AUX_DIR([build-aux])
# The following sets CFLAGS and CXXFLAGS to empty if unset on command line.
: ${CFLAGS=""}
: ${CXXFLAGS=""}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd get rid of this. It doesn't hurt anything, but you aren't using an C++ compilers. Even if you set CC=clang++, configure will treat clang++ as a C compiler and only use the CFLAGS.

@@ -227,7 +227,6 @@
])

AC_DEFUN([AX_CC_OTHER_FLAGS], [
AC_REQUIRE([AX_APPEND_COMPILE_FLAGS])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. I need to fix this over at wolfSSH. AC_REQUIRE forces use of the macro, not checking if it is present.

configure.ac Outdated
@@ -52,8 +54,8 @@ AC_PROG_INSTALL

# Checks for header files.
AC_HEADER_STDC
AC_CHECK_SIZEOF([long long], 8)
AC_CHECK_SIZEOF([long], 4)
AC_CHECK_SIZEOF(long long, 8)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The brackets should be there. The parameters to the macros should be quoted with the brackets. Also, AC_CHECK_SIZEOF only takes a single parameter, the type name. The 4 and 8 are ignored.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or the brackets don't hurt. They aren't necessary in every case, but they work in every case. I found it easier to put them in then to try to remember the exception cases.

configure.ac Outdated
AC_INIT([wolfmqtt],[1.0.0],[https://github.com/wolfssl/wolfMQTT/issues],[wolfmqtt],[http://www.wolfssl.com])

AC_PREREQ([2.63])
AC_CONFIG_AUX_DIR([build-aux])

: ${CFLAGS=""}
: ${AR_FLAGS="cr"}

AC_CANONICAL_HOST
AC_CANONICAL_BUILD
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The autoconf manual states that you should only use one of AC_CANONICAL_HOST, AC_CANONICAL_BUILD, or AC_CANONICAL_TARGET. wolfSSL has listed both, and only the HOST is expanded in the configure script, BUILD has been ignored. (I'm not sure which one we really need, wolfSSL has always had two of them so we've always had a "canonical host" since Brian added autotools.)

@ejohnstown ejohnstown dismissed their stale review May 23, 2018 20:00

Things changed, and I had comments on them.

…Cleanup the `AC_CHECK_FUNCS` and `AC_CHECK_LIB`.
@ejohnstown ejohnstown merged commit ada12af into wolfSSL:master May 23, 2018
@dgarske dgarske deleted the autoconfm4 branch May 23, 2018 20:35
kojo1 pushed a commit to kojo1/wolfMQTT that referenced this pull request Aug 21, 2022
LPC: Fix aligned page address write
This pull request was closed.
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.

3 participants