Re: [Powertop] Complete build options for Pthread API

[SF Markus Elfring <elfring at users.sourceforge.net>]

[-- Attachment #1: Type: text/plain, Size: 2431 bytes --]

> I don't know what sort of adjustments you have in mind.

I suggest a bit more fine-tuning for the explanation.


> The variables are in commit body, so I am assuming people who read code
> can see them without explicit note about them.

I would prefer a commit message that will also be useful to some degree without
reading the affected source code.


> But if you insist improvement with the message could you write what you
> would like to see so that I can include it without confusions.

You can see from a few contributions for other software that I used a wording
like the following:

The configuration script "AX_PTHREAD" was added to the build specification
so that corresponding special parameters will be taken into account.
http://www.gnu.org/software/autoconf-archive/ax_pthread.html



You might be still interested to stress other details for your PowerTop testers
and reviewers.



>>> +AX_PTHREAD([], [AC_MSG_ERROR([Could not configure pthreads support])])
>>
>> 2. You chose to pass an empty "ACTION-IF-FOUND". Would it make sense to
>> restructure this place a bit in the configuration script so that the
>> following statements will be directly used by the first macro parameter?
> 
> That would make powertop m4 code copy to drift from version provided by
> autotools-archive, making the upstream fixes to incompatible. I strongly
> recommend against adding additional maintenance cost to this sort of stuff.

I do not really understand the intention in this feedback.


How do you think about the following statement layout at the discussed script place?

diff --git a/configure.ac b/configure.ac
index 4991d97..0ebe69d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -128,11 +128,12 @@ AS_IF([test "$has_libpci" -eq 0], [
 	])
 ])

-AC_SEARCH_LIBS([pthread_create], [pthread], [], [
-	AC_MSG_ERROR([libpthread is required but was not found])
-], [])
-AC_SEARCH_LIBS([inet_aton], [resolv], [], [
-	AC_MSG_ERROR([libresolv is required but was not found])
-], [])
-
-AC_OUTPUT
+AX_PTHREAD([LIBS="$PTHREAD_LIBS $LIBS"
+CFLAGS="$CFLAGS $PTHREAD_CFLAGS"
+CC="$PTHREAD_CC"
+AC_SEARCH_LIBS([inet_aton],
+               [resolv],
+               [AC_OUTPUT],
+               [AC_MSG_ERROR([libresolv is required but was not found])],
+               [])],
+           [AC_MSG_ERROR([Could not configure pthreads support])])


Regards,
Markus