Bug 13629 - libtevent does not build due to missing open_memstream on illiumos
libtevent does not build due to missing open_memstream on illiumos
Status: RESOLVED FIXED
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Build
4.9.0
All Solaris
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2018-09-22 21:37 UTC by Jorge Schrauwen
Modified: 2018-11-20 11:21 UTC (History)
2 users (show)

See Also:


Attachments
git-am fix for 4.9.next. (5.66 KB, patch)
2018-10-08 23:59 UTC, Jeremy Allison
vl: review+
Details
Fix crash (1007 bytes, patch)
2018-11-12 15:24 UTC, Volker Lendecke
bbaumbach: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jorge Schrauwen 2018-09-22 21:37:55 UTC
I just tried to build 4.9.0 which now fails on illumos due to not having open_memstream.

As far as I can tell there is only one call that uses that in lib/util/tevent_req_profile.c which was added in this release.

It does not look possible to compile without this.
Comment 1 Jeremy Allison 2018-09-22 21:48:39 UTC
open_memstream() is a POSIX standard function:

http://pubs.opengroup.org/onlinepubs/9699919799/functions/open_memstream.html

Does Illumos not follow POSIX anymore ?
Comment 2 Jorge Schrauwen 2018-09-22 21:59:58 UTC
Looks like at least this one is not implemented, there is a feature request open for it but it has been so for 2 years.

https://www.illumos.org/issues/7092
Comment 3 Jeremy Allison 2018-10-08 23:59:22 UTC
Created attachment 14518 [details]
git-am fix for 4.9.next.

Cherry-picked from master.
Comment 4 Jorge Schrauwen 2018-10-09 12:51:44 UTC
I quickly chocked this over 4.9.1 and I now get a compilation error...

In file included from ../lib/util/tevent_req_profile.c:26:0:
../lib/replace/replace.h:823:0: warning: "ARRAY_SIZE" redefined
 #define ARRAY_SIZE(a) (sizeof(a)/sizeof(a[0]))
 ^
In file included from ../lib/replace/replace.h:175:0,
                 from ../lib/util/tevent_req_profile.c:26:
/usr/include/sys/sysmacros.h:373:0: note: this is the location of the previous definition
 #define ARRAY_SIZE(x) (sizeof (x) / sizeof (x[0]))
 ^
../lib/util/tevent_req_profile.c:133:7: error: conflicting types for 'tevent_req_profile_string'
 char *tevent_req_profile_string(TALLOC_CTX *mem_ctx,
       ^
In file included from ../lib/util/tevent_req_profile.c:28:0:
../lib/util/tevent_req_profile.h:36:7: note: previous declaration of 'tevent_req_profile_string' was here
 char *tevent_req_profile_string(const struct tevent_req_profile *profile,
       ^

The ARRAY_SIZE warning has been around for a long time and seems harmless but now it complains about conflicting types in "char *tevent_req_profile_string(TALLOC_CTX *mem_ctx,"

Not sure if the linked patched missed a file or if I messed up. I am traveling this and next week so a quick and dirty apply patch and compile.
Comment 5 Volker Lendecke 2018-10-09 13:57:12 UTC
(In reply to Jorge Schrauwen from comment #4)
> Not sure if the linked patched missed a file or if I messed up. I am
> traveling this and next week so a quick and dirty apply patch and compile.

The linked patch looks fine to me, I compiled it under Linux without a warning about an incorrect prototype. I think you should wait until 4.9.2 is out to avoid problems with the patch process under Solaris.
Comment 6 Volker Lendecke 2018-10-09 13:58:19 UTC
Bug reporter claims fix does not work. We should wait until master trickles into 4.10 and not mess with a production release.
Comment 7 Jorge Schrauwen 2018-10-09 15:25:24 UTC
Just to double check, you advise to just not build any of the future 4.9.x release on Solaris/Illumos and wait for a future 4.10 release?

The 2 too comments contradict each other.
Comment 8 Volker Lendecke 2018-10-09 15:31:42 UTC
Correct. I came to the conclusion that because the patch does not help for 4.9, in order to not destabilize 4.9 we should just skip that one. As 4.10 will come soon and 4.8 is still in full support until 4.10, it's better to just wait.
Comment 9 Jeremy Allison 2018-10-09 17:20:39 UTC
Jorge, I think you messed up applying the patch. Please re-sync with 4.9.x and try again. At least locally here the patch seems good and applies cleanly with no prototype errors.

Let's not close immediately as the HPUX folks also suffer from this, so it would be good to get it into 4.9.next if the submitter can confirm.
Comment 10 Jorge Schrauwen 2018-10-09 18:38:34 UTC
You were right Jeremy Allison,

I was doing this on the bus, now I am back at my hotel I noticed I forgot to patch the header. With that also applied the build succeeds, I have not upgrade my server yet but the patch looks fine!

Waf: Leaving directory `/home/pbulk/build/pbd/samba-pbd/work/samba-4.9.1/bin'
'build' finished successfully (5m12.211s)
Comment 11 Jeremy Allison 2018-10-09 18:47:40 UTC
Now submitter has confirmed patch is good (and Volker already +1'ed) - re-assigning to Karolin for inclusion in 4.9.next.
Comment 12 Karolin Seeger 2018-10-10 09:07:06 UTC
(In reply to Jeremy Allison from comment #11)
Pushed to autobuild-v4-9-test.
Comment 13 Karolin Seeger 2018-10-11 10:13:33 UTC
(In reply to Karolin Seeger from comment #12)
Pushed to v4-9-test.
Closing out bug report.

Thanks!
Comment 14 Volker Lendecke 2018-11-12 15:24:09 UTC
Created attachment 14643 [details]
Fix crash

source3/winbindd/winbindd.c's call to tevent_req_profile() now has the first two parameters swapped, because I changed them in lib/util/tevent_req_profile.[ch] but forgot winbindd.c.
Comment 15 Volker Lendecke 2018-11-12 15:39:24 UTC
Karolin, can you apply the additional patch? It fixes a winbind crash.

Sorry, Volker
Comment 16 Jorge Schrauwen 2018-11-12 17:32:10 UTC
Still works fine with the 2nd patch also applied on illumos.
Comment 17 Karolin Seeger 2018-11-14 11:28:53 UTC
(In reply to Volker Lendecke from comment #15)
Sure, pushed to autobuild-v4-9-test.
Comment 18 Karolin Seeger 2018-11-20 11:21:01 UTC
(In reply to Karolin Seeger from comment #17)
Pushed to v4-9-test.
Closing out bug report.

Thanks!