Skip to content

configure: also check for clockgettime()#9

Merged
octo merged 1 commit into
octo:masterfrom
yann-morin-1998:yem/fixes
Jul 15, 2016
Merged

configure: also check for clockgettime()#9
octo merged 1 commit into
octo:masterfrom
yann-morin-1998:yem/fixes

Conversation

@yann-morin-1998

Copy link
Copy Markdown
Contributor

clock_gettime() is also in -lrt so we also need to check for it.

Signed-off-by: "Yann E. MORIN" yann.morin.1998@free.fr

@yann-morin-1998

Copy link
Copy Markdown
Contributor Author

I just forgot a little nit in the previous iteration, so I just repushed a fixed patch.

@octo

octo commented Jul 14, 2016

Copy link
Copy Markdown
Owner

Hi @yann-morin-1998, thank you very much for your PR! I'll leave some review comments inline.

Best regards,
—octo

Comment thread configure.ac Outdated
[needs_rt="yes"],
AC_MSG_ERROR(cannot find nanosleep)))
AM_CONDITIONAL(BUILD_WITH_LIBRT, test "x$nanosleep_needs_rt" = "xyes")
AC_CHECK_FUNCS(clock_gettime, [],

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you do the same once more for nanosleep()? I have the feeling that clock_gettime() might need librt more often than nanosleep(), leading to compilation errors on systems where only nanosleep() needs the lib.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you change this to AC_CHECK_FUNC() (no trailing 'S') please? IIrc the generated shell code for checking multiple functions is quite substantial.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hello, thanks for the feedback! :-)

So far, I just duplicated the nanosleep() case to test for clock_gettime().

However, now that I re-read the autoconf manual, here's what I see:

If the functions might be in libraries other than the default C
library, first call AC_CHECK_LIB for those libraries.

So, to me, we are doing it backwards. We should probably just use
AC_SEARCH_LIBS() and leave it to autoconf to properly add -lrt to
LIBS if needed. Something like:

AC_SEARCH_LIBS([nanosleep],[rt],,[AC_MSG_ERROR([cannot find nanosleep])])
AC_SEARCH_LIBS([clock_gettime],[rt],,[AC_MSG_ERROR([cannot find nanosleep])])

And then get rid of the AM_CONDITIONAL() stuff, since LIBS will
contain -lrt if needed by either.

Thoughts?

@octo octo added the Bugfix label Jul 14, 2016
clock_gettime() is also in -lrt so we also need to
check for it.

Use AC_SEARCH_LIBS() instead of our canned combo of
AC_CHECK_FUNC() + AC_CHECK_LIB(). AC_SEARCH_LIBS()
will automatically add the necessary -l flags to the
LIBS variable, so we don't need out AM_CONDITIONAL()
construct either, now.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
@yann-morin-1998

Copy link
Copy Markdown
Contributor Author

I've re-pushed the commit using AC_SEARCH_LIBS() now.

In my test-case (fixing http://autobuild.buildroot.org/results/6a9/6a9b4d7b1b3cd72e32579fbaf5a69dbde0fea8e4/), it works as
expected:

checking for library containing nanosleep... none required
checking for library containing clock_gettime... -lrt

Regards,
Yann E. MORIN.

@octo

octo commented Jul 15, 2016

Copy link
Copy Markdown
Owner

Thanks Yann! Let's see if this is doing the right thing – it might be that this will now also link liboping with librt (in addition to the oping and noping binaries). I'll worry about that when someone complaints though ;)

Best regards,
—octo

@octo octo merged commit 6e83f8b into octo:master Jul 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants