tp_test.c: fix compilation on Solaris

grobian commented 6 years ago

On Solaris, getopt(3) is in stdio.h.

grobian commented 6 years ago

I have the impression the travis failures are due to AS_ROOT=yes. I can't see an obvious error in the buildlogs, but I may not know where to look.

ngie-eign commented 5 years ago

@grobian: I disagree, per the current Solaris spec: https://docs.oracle.com/cd/E86824_01/html/E54766/getopt-3c.html . Is Solaris compiling with non-POSIX mode enabled?

grobian commented 5 years ago

I assumed you wouldn't want that, as that would require trickery with either the C-standard or _POSIX_SOURCE feature tests, which on Solaris requires you to do it correctly, else you get an error. Just including stdio.h seemed safer and less hassle ;)

ngie-eign commented 5 years ago

I assumed you wouldn't want that, as that would require trickery with either the C-standard or _POSIX_SOURCE feature tests, which on Solaris requires you to do it correctly, else you get an error. Just including stdio.h seemed safer and less hassle ;)

Not really. Adding AC_USE_SYSTEM_EXTENSIONS to configure.ac would do the trick IMHO.

grobian commented 5 years ago

worth a shot

grobian commented 5 years ago

atf-c/tp_test.c: In function ‘atfu_getopt_body’:
atf-c/tp_test.c:59:18: error: implicit declaration of function ‘getopt’; did you mean ‘gettxt’? [-Werror=implicit-function-declaration]
     while ((ch = getopt(argc, argv, ":Z")) != -1) {
                  ^~~~~~
                  gettxt
atf-c/tp_test.c:67:17: error: ‘optopt’ undeclared (first use in this function)
             if (optopt != 's')
                 ^~~~~~
atf-c/tp_test.c:67:17: note: each undeclared identifier is reported only once for each function it appears in
In file included from ./atf-c.h:30,
                 from atf-c/tp_test.c:31:
atf-c/tp_test.c:73:34: error: ‘optind’ undeclared (first use in this function)
     ATF_REQUIRE_EQ_MSG(1, argc - optind, "Invalid number of arguments left "
                                  ^~~~~~
./atf-c/macros.h:132:15: note: in definition of macro ‘ATF_REQUIRE_MSG’
         if (!(expression)) \
               ^~~~~~~~~~
atf-c/tp_test.c:73:5: note: in expansion of macro ‘ATF_REQUIRE_EQ_MSG’
     ATF_REQUIRE_EQ_MSG(1, argc - optind, "Invalid number of arguments left "
     ^~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
make[1]: *** [Makefile:2642: atf-c/tp_test-tp_test.o] Error 1

The reason for this is obviously because atf-c/tp_test.c doesn't include config.h. Adding the include next to AC_USE_SYSTEM_EXTENSIONS makes the utility compile.

So it's either those changes, or addition of stdio.h as in my original patch.

jmmv commented 5 years ago

@googlebot rescan

(Mass-triggering a rescan of all open PRs as I have just configured automated CLA checks but the bot won't go through the backlog on its own.)

googlebot commented 5 years ago

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

:memo: Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

ngie-eign commented 7 months ago

Hey @grobian ! I think adding config.h to the file and the autoconf magic is the right thing to do in this case. If you do that, I can take the change. Thanks!

grobian commented 7 months ago

@ngie-eign is this what you had in mind?

ngie-eign commented 5 months ago

@ngie-eign is this what you had in mind?

Yup -- that's perfect :)!

I wouldn't have pushed it directly to a master branch if I had been you, but it's ok. Water under the bridge :).

freebsd / atf